On Tue, Jun 07, 2016 at 08:51:27AM +0200, Theo Buehler wrote:
> On Sun, May 29, 2016 at 10:55:48PM +0200, Theo Buehler wrote:
> > The readlabel() function in disklabel() does two things: it reads the
> > disklabel from the device using a ioctl() and then parses it into some
> > strings.  We can't pledge beforehand since we have no way of knowing the
> > file we process is actually a disk device.  However, once the ioctl()
> > succeeds, we know that we deal with a disk and we can do all further
> > processing of the untrusted data under pledge.
> > 
> > Thus, split up readlabel() into two functions and pledge between the
> > two function calls.
> 
> Here's an updated version of this diff that also takes care of two
> further things:
> 
> First, we need to call parse_autotable() before parselabel() because the
> template will be used by the call to editor_allocspace() in parselabel().
> 
> Second, there still is a pledge problem:
> 
> $ ktrace disklabel -w /dev/tty floppy576
> Abort trap (core dumped)

Here's the third revision after jsg@'s fix went in.

In the (op == WRITE && (argc == 2 || argc == 3)) path, makelabel() will
not honor or need an autotable and the argument parsing will only allow
-[fF] with -E or -R, so we can keep the simple pledge logic there.

Index: disklabel.c
===================================================================
RCS file: /var/cvs/src/sbin/disklabel/disklabel.c,v
retrieving revision 1.221
diff -u -p -r1.221 disklabel.c
--- disklabel.c 13 Jun 2016 09:54:01 -0000      1.221
+++ disklabel.c 13 Jun 2016 10:12:46 -0000
@@ -206,26 +206,36 @@ main(int argc, char *argv[])
        if (f < 0)
                err(4, "%s", specname);
 
-       if (autotable != NULL)
-               parse_autotable(autotable);
-
-       if (op != WRITE || aflag || dflag)
+       if (op != WRITE || aflag || dflag) {
                readlabel(f);
-       else if (argc == 2 || argc == 3)
-               makelabel(argv[1], argc == 3 ? argv[2] : NULL, &lab);
-       else
-               usage();
 
-       if (op == EDIT || op == EDITOR || aflag) {
-               if (pledge("stdio rpath wpath cpath disklabel proc exec", NULL) 
== -1)
-                       err(1, "pledge");
-       } else if (fstabfile) {
-               if (pledge("stdio rpath wpath cpath disklabel", NULL) == -1)
-                       err(1, "pledge");
-       } else {
+               if (op == EDIT || op == EDITOR || aflag) {
+                       if (pledge("stdio rpath wpath cpath disklabel proc "
+                           "exec", NULL) == -1)
+                               err(1, "pledge");
+               } else if (fstabfile) {
+                       if (pledge("stdio rpath wpath cpath disklabel", NULL)
+                           == -1)
+                               err(1, "pledge");
+               } else {
+                       if (pledge("stdio rpath wpath disklabel", NULL) == -1)
+                               err(1, "pledge");
+               }
+
+               if (autotable != NULL)
+                       parse_autotable(autotable);
+               parselabel();
+       } else if (argc == 2 || argc == 3) {
+               /* Ensure f is a disk device before pledging. */
+               if (ioctl(f, DIOCGDINFO, &lab) < 0)
+                       err(4, "ioctl DIOCGDINFO");
+
                if (pledge("stdio rpath wpath disklabel", NULL) == -1)
                        err(1, "pledge");
-       }
+
+               makelabel(argv[1], argc == 3 ? argv[2] : NULL, &lab);
+       } else
+               usage();
 
        switch (op) {
        case EDIT:
@@ -356,9 +366,6 @@ l_perror(char *s)
 void
 readlabel(int f)
 {
-       char *partname, *partduid;
-       struct fstab *fsent;
-       int i;
 
        if (cflag && ioctl(f, DIOCRLDINFO) < 0)
                err(4, "ioctl DIOCRLDINFO");
@@ -370,6 +377,14 @@ readlabel(int f)
                if (ioctl(f, DIOCGDINFO, &lab) < 0)
                        err(4, "ioctl DIOCGDINFO");
        }
+}
+
+void
+parselabel(void)
+{
+       char *partname, *partduid;
+       struct fstab *fsent;
+       int i;
 
        i = asprintf(&partname, "/dev/%s%c", dkname, 'a');
        if (i == -1)
Index: extern.h
===================================================================
RCS file: /var/cvs/src/sbin/disklabel/extern.h,v
retrieving revision 1.27
diff -u -p -r1.27 extern.h
--- extern.h    17 Oct 2015 13:27:08 -0000      1.27
+++ extern.h    7 Jun 2016 06:18:50 -0000
@@ -28,6 +28,7 @@ void  display_partition(FILE *, struct di
 int    duid_parse(struct disklabel *, char *);
 
 void   readlabel(int);
+void   parselabel(void);
 struct disklabel *makebootarea(char *, struct disklabel *);
 int    editor(int);
 void   editor_allocspace(struct disklabel *);

Reply via email to