Just forgot to pass the disk during sparc64 tests and was surprised to
see it treating a file as disk:

        # installboot -nv /usr/mdec/bootblk /ofwboot.test
        Using / as root
        would install bootstrap on ./usr/mdec/bootblk
        using first-stage ./ofwboot.test, second-stage /usr/mdec/ofwboot
        boot block is 120696 bytes (236 blocks @ 512 bytes = 120832 bytes)
        installboot: boot blocks too big (120832 > 7680)

Same on amd64:

        $ installboot -nv /usr/mdec/biosboot /usr/mdec/boot      
        Using / as root
        would install bootstrap on /usr/mdec/biosboot
        using first-stage /usr/mdec/boot, second-stage /usr/mdec/boot
        installboot: /usr/mdec/boot: 2 ELF load sections (only support 1)

Require an exact argc match instead of erroring out too many args alone:

        $ ./obj/installboot -nv /usr/mdec/biosboot /usr/mdec/boot
        usage:  installboot [-nv] [-r root] disk [stage1 [stage2]]
                installboot [-nv] -p disk


This problem exists on all platforms, but EFI (armv7, arm64, risc64)
currently suffers from another bug regarding an explicit stage file:

        $ installboot -nv sd0 /usr/mdec/BOOTAA64.EFI 
        usage:  installboot [-nv] [-r root] disk [stage1]
                installboot [-nv] -p disk

There md_init() does not yet set `stages' (relevant for this argc fix)
and `stage1', so `stage' remains zero-initialised, effectively always
requiring exactly one argument (the disk).

That's stuff for another diff and luckily the argc fix below does not
change behaviour for the only currently working usage on EFI:

        # doas installboot -nv sd0
        Using / as root
        would install bootstrap on /dev/rsd0c
        would copy /usr/mdec/BOOTAA64.EFI to 
/tmp/installboot.7w8t7zd34s/efi/boot/bootaa64.efi
        would write /tmp/installboot.7w8t7zd34s/efi/boot/startup.nsh
        # doas ./obj/installboot -nv sd0
        Using / as root
        would install bootstrap on /dev/rsd0c
        would copy /usr/mdec/BOOTAA64.EFI to 
/tmp/installboot.OMturEqYaM/efi/boot/bootaa64.efi
        would write /tmp/installboot.OMturEqYaM/efi/boot/startup.nsh


The argc checks now read like I would read the synopsis, making it much
easier to read and understand than the current code:
1. -p?
  a. no -r?
  b. exactly one arg (disk)?
2. else
  a. exactly one arg (disk)?
  b. exactly one+stages args (disk + stage1 [+ stage2])?


Pull stage1/stage2 assignments into 2. to clarify things further.

This fixes the bogus case I hit and keeps existing valid usages working.

Feedback? OK?


Index: installboot.c
===================================================================
RCS file: /cvs/src/usr.sbin/installboot/installboot.c,v
retrieving revision 1.15
diff -u -p -r1.15 installboot.c
--- installboot.c       19 Aug 2022 08:27:48 -0000      1.15
+++ installboot.c       20 Aug 2022 06:29:49 -0000
@@ -75,10 +75,13 @@ main(int argc, char **argv)
        argc -= optind;
        argv += optind;
 
-       if (argc < 1 || argc > stages + 1)
-               usage();
-       if (prepare && (root != NULL || argc > 1))
-               usage();
+       if (prepare) {
+               if (root != NULL || argc != 1)
+                       usage();
+       } else {
+               if (argc != 1 && argc != stages + 1)
+                       usage();
+       }
 
        dev = argv[0];
        if (argc > 1)

Reply via email to