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 *);