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)
$ kdump | tail
36196 disklabel RET mmap 22796710084608/0x14bbc5ce8000
36196 disklabel CALL
mmap(0,0x1000,0x3<PROT_READ|PROT_WRITE>,0x1002<MAP_PRIVATE|MAP_ANON>,-1,0)
36196 disklabel RET mmap 22795116298240/0x14bb66cf4000
36196 disklabel CALL pledge(0x14b92f938483,0)
36196 disklabel STRU pledge promise="stdio rpath wpath disklabel"
36196 disklabel RET pledge 0
36196 disklabel CALL ioctl(3,DIOCWDINFO,0x14b92fc4c180)
36196 disklabel PLDG ioctl, "ioctl", errno 1 Operation not permitted
36196 disklabel PSIG SIGABRT SIG_DFL
36196 disklabel NAMI "disklabel.core"
Since the DIOC{W,G}DINFO ioctls are called very late in the
(op == WRITE && (argc == 2 || argc == 3)) codepath, we either need to
drop pledge in that case or we can interpolate a dummy DIOCGDINFO call
that ensures the opened device is a disk device. We can then pledge
before parsing disktab(5). I went for the second option.
Index: disklabel.c
===================================================================
RCS file: /var/cvs/src/sbin/disklabel/disklabel.c,v
retrieving revision 1.220
diff -u -p -r1.220 disklabel.c
--- disklabel.c 1 Jun 2016 16:51:54 -0000 1.220
+++ disklabel.c 7 Jun 2016 06:15:13 -0000
@@ -206,23 +206,32 @@ 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 (op == EDIT || op == EDITOR || aflag) {
+ if (pledge("stdio rpath wpath cpath disklabel proc "
+ "exec", 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:
@@ -353,9 +362,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");
@@ -367,6 +373,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 6 Jun 2016 19:16:37 -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 *);
