On Mon, Jun 01, 2020 at 11:48:05PM +0200, Klemens Nanni wrote:
> Installing an unstripped boot loader on softraid on sparc64 fails
> without proper error message.
> 
> Make BIOCINSTALLBOOT return a proper errno, make installboot(8) use it
> to provide proper usage errors plus unique code paths for debugging.
> 
> At first, I made sr_ioctl_installboot() use sr_error() in the relevant
> spot and this made the kernel print my errors, however the following
> piece in softraid.c:sr_bio_handler() means using sr_error() will hide
> the error code from the ioctl(2) call, i.e. installboot(8) would
> report no error message on stderr and exit zero:
> 
>       if (sc->sc_status.bs_msg_count > 0)                  
>               rv = 0;    
> 
> So instead, use a proper errno that yields a simple
> 
>       # ./obj/installboot sd2 /usr/mdec/bootblk /ofwboot.new
>       installboot.sr: softraid installboot failed: File too large
> 
> 
> 
> Background:  I built ofwboot on one machine ("make" without
> "make install", then copy obj/ofwboot over), the resulting executable
> was not stripped during build (happens during "make install") and was
> about twice as big:
> 
>       # ls -l /ofwboot*       
>       -rw-r--r--  1 root  wheel  106688 May 23 22:42 /ofwboot
>       -rwxr-xr-x  1 kn    wheel  272452 May 24 00:20 /ofwboot.new
>       -rwxr-xr-x  1 root  wheel  106752 May 24 01:21 /ofwboot.new.strip
> 
> It took me longer than anticipated to find out that installboot(8)
> fails beause my new boot loader was too big:
> 
>       # installboot sd2 /usr/mdec/bootblk /ofwboot.new
>       installboot: softraid installboot failed
> 
>       # installboot -v sd2 /usr/mdec/bootblk /ofwboot.new
>       Using / as root
>       installing bootstrap on /dev/rsd2c
>       using first-stage /usr/mdec/bootblk, second-stage /ofwboot.new
>       boot block is 6882 bytes (14 blocks @ 512 bytes = 7168 bytes)
>       sd2: softraid volume with 1 disk(s)
>       sd2: installing boot loader on softraid volume
>       installboot: softraid installboot failed
> 
> That's it, no details or additional messages from the kernel.
> While this was primarily my own fault, perhaps there are more legitimate
> reasons foor bootblk/ofwboot builds to exceed their maximum size.

In a i386 VM with root on crypto softraid, I built a much bigger second
stage boot loader and performed the same tests as on sparc64: i386 does
not try to install the bogus boot code due to checks in i386_softraid.c
up front:

        # ls -l /usr/mdec/boot /boot.efbig
        -r-xr-xr-x  1 root  bin     89728 Jun  5 03:02 /usr/mdec/boot
        -rwxr-xr-x  1 root  wheel  176172 Jun  5 22:16 /boot.efbig
        # installboot -v sd1 /usr/mdec/biosboot /boot.efbig
        Using / as root
        installing bootstrap on /dev/rsd1c
        using first-stage /usr/mdec/biosboot, second-stage /boot.efbig
        sd1: softraid volume with 1 disk(s)
        installboot: boot code will not fit

So for installboot(8) on sparc64 and i386 as the only two users of
BIOCINSTALLBOOT, this makes root on softraid on sparc64 the only use
that actually hits size checks in the ioctl code.


sr_ioctl_installboot() seems inconsistent to me in how it reports some
errors through sr_error() (causing ioctl() to return zero) and returing
proper error codes (causing ioctl() and therefore installboot to fail);

Assuming my EFBIG approach is still sensible (for sparc64), diff below
adjusts the errx() call to err() in installboot for both platforms, even
though i386 never reaches it.

Feedback? OK?


Index: sys/dev/softraid.c
===================================================================
RCS file: /cvs/src/sys/dev/softraid.c,v
retrieving revision 1.401
diff -u -p -r1.401 softraid.c
--- sys/dev/softraid.c  14 Apr 2020 07:38:21 -0000      1.401
+++ sys/dev/softraid.c  5 Jun 2020 20:41:20 -0000
@@ -3704,11 +3704,11 @@ sr_ioctl_installboot(struct sr_softc *sc
                goto done;
        }
 
-       if (bb->bb_bootblk_size > SR_BOOT_BLOCKS_SIZE * DEV_BSIZE)
-               goto done;
-
-       if (bb->bb_bootldr_size > SR_BOOT_LOADER_SIZE * DEV_BSIZE)
+       if (bb->bb_bootblk_size > SR_BOOT_BLOCKS_SIZE * DEV_BSIZE ||
+           bb->bb_bootldr_size > SR_BOOT_LOADER_SIZE * DEV_BSIZE) {
+               rv = EFBIG;
                goto done;
+       }
 
        secsize = sd->sd_meta->ssdi.ssd_secsize;
 
Index: usr.sbin/installboot/i386_softraid.c
===================================================================
RCS file: /cvs/src/usr.sbin/installboot/i386_softraid.c,v
retrieving revision 1.15
diff -u -p -r1.15 i386_softraid.c
--- usr.sbin/installboot/i386_softraid.c        9 Mar 2020 06:16:56 -0000       
1.15
+++ usr.sbin/installboot/i386_softraid.c        5 Jun 2020 20:39:54 -0000
@@ -177,7 +177,7 @@ sr_install_bootldr(int devfd, char *dev)
                        fprintf(stderr, "%s: installing boot loader on "
                            "softraid volume\n", dev);
                if (ioctl(devfd, BIOCINSTALLBOOT, &bb) == -1)
-                       errx(1, "softraid installboot failed");
+                       err(1, "softraid installboot failed");
        }
 
        /*
Index: usr.sbin/installboot/sparc64_softraid.c
===================================================================
RCS file: /cvs/src/usr.sbin/installboot/sparc64_softraid.c,v
retrieving revision 1.4
diff -u -p -r1.4 sparc64_softraid.c
--- usr.sbin/installboot/sparc64_softraid.c     28 Jun 2019 13:32:48 -0000      
1.4
+++ usr.sbin/installboot/sparc64_softraid.c     5 Jun 2020 20:40:05 -0000
@@ -96,6 +96,6 @@ sr_install_bootldr(int devfd, char *dev)
                        fprintf(stderr, "%s: installing boot loader on "
                            "softraid volume\n", dev);
                if (ioctl(devfd, BIOCINSTALLBOOT, &bb) == -1)
-                       errx(1, "softraid installboot failed");
+                       err(1, "softraid installboot failed");
        }
 }

Reply via email to