Reflect script failure in exit code

installboot(8) runs newfs(8) and fsck(8) via system(3) but only checks
failures of the function itself, always returning zero no matter what the
programs/shell returned.

This is bad for regress tests relying on correct return codes.

create_filesystem() itself must not exit as write_filesystem() calls it and
cleans up temporary files upon failure.

Make it return -1 if the script returned non-zero so write_filesystem()
handles it as error, cleans up and makes installboot exit 1.

Stop ignoring create_filesystem()'s return code in md_prepareboot() and
exit the same way.

Here's the change in behaviour on arm64 (newfs fails because of the
vnd/disklabel race, see "Race in disk_attach_callback?" on tech@):

	# installboot -vp vnd0 ; echo $?
	newfsing 6694ae5b0d7596ed.i
	newfs_msdos: /dev/r6694ae5b0d7596ed.i: No such file or directory
	0
	# ./obj/installboot -vp vnd0 ; echo $?
	newfsing 6694ae5b0d7596ed.i
	newfs_msdos: /dev/r6694ae5b0d7596ed.i: No such file or directory
	1

Tested on amd64 arm64 macppc octeon powerpc64 sparc64
OK millert

diff --git a/usr.sbin/installboot/efi_installboot.c b/usr.sbin/installboot/efi_installboot.c
index eb3c311..54142ce 100644 (file)
--- a/usr.sbin/installboot/efi_installboot.c
+++ b/usr.sbin/installboot/efi_installboot.c
@@ -1,4 +1,4 @@
-/*     $OpenBSD: efi_installboot.c,v 1.4 2022/09/07 10:21:03 kn Exp $  */
+/*     $OpenBSD: efi_installboot.c,v 1.5 2022/09/09 15:53:16 kn Exp $  */
 /*     $NetBSD: installboot.c,v 1.5 1995/11/17 23:23:50 gwr Exp $ */
 
 /*
@@ -41,6 +41,7 @@
 #include <sys/ioctl.h>
 #include <sys/mount.h>
 #include <sys/stat.h>
+#include <sys/wait.h>
 
 #include <err.h>
 #include <errno.h>
@@ -102,15 +103,11 @@ md_prepareboot(int devfd, char *dev)
                warnx("disklabel type unknown");
 
        part = findgptefisys(devfd, &dl);
+       if (part == -1)
+               part = findmbrfat(devfd, &dl);
        if (part != -1) {
-               create_filesystem(&dl, (char)part);
-               return;
-       }
-
-       part = findmbrfat(devfd, &dl);
-       if (part != -1) {
-               create_filesystem(&dl, (char)part);
-               return;
+               if (create_filesystem(&dl, (char)part) == -1)
+                       exit(1);
        }
 }
 
@@ -179,6 +176,8 @@ create_filesystem(struct disklabel *dl, char part)
                        warn("system('%s') failed", cmd);
                        return rslt;
                }
+               if (WIFEXITED(rslt) && WEXITSTATUS(rslt))
+                       return -1;
        }
 
        return 0;
@@ -232,6 +231,10 @@ write_filesystem(struct disklabel *dl, char part)
                        warn("system('%s') failed", cmd);
                        goto rmdir;
                }
+               if (WIFEXITED(rslt) && WEXITSTATUS(rslt)) {
+                       rslt = -1;
+                       goto rmdir;
+               }
                if (mount(MOUNT_MSDOS, dst, 0, &args) == -1) {
                        /* Try newfs'ing it. */
                        rslt = create_filesystem(dl, part);

diff --git a/usr.sbin/installboot/i386_installboot.c b/usr.sbin/installboot/i386_installboot.c
index 84dbb8f..122bc16 100644 (file)
--- a/usr.sbin/installboot/i386_installboot.c
+++ b/usr.sbin/installboot/i386_installboot.c
@@ -1,4 +1,4 @@
-/*     $OpenBSD: i386_installboot.c,v 1.41 2022/08/31 20:52:15 krw Exp $       */
+/*     $OpenBSD: i386_installboot.c,v 1.42 2022/09/09 15:53:16 kn Exp $        */
 /*     $NetBSD: installboot.c,v 1.5 1995/11/17 23:23:50 gwr Exp $ */
 
 /*
@@ -46,6 +46,7 @@
 #include <sys/stat.h>
 #include <sys/sysctl.h>
 #include <sys/time.h>
+#include <sys/wait.h>
 
 #include <ufs/ufs/dinode.h>
 #include <ufs/ufs/dir.h>
@@ -146,8 +147,8 @@ md_prepareboot(int devfd, char *dev)
 
        part = findgptefisys(devfd, &dl);
        if (part != -1) {
-               create_filesystem(&dl, (char)part);
-               return;
+               if (create_filesystem(&dl, (char)part) == -1)
+                       exit(1);
        }
 }
 
@@ -280,6 +281,8 @@ create_filesystem(struct disklabel *dl, char part)
                        warn("system('%s') failed", cmd);
                        return rslt;
                }
+               if (WIFEXITED(rslt) && WEXITSTATUS(rslt))
+                       return -1;
        }
 
        return 0;
@@ -333,6 +336,10 @@ write_filesystem(struct disklabel *dl, char part)
                        warn("system('%s') failed", cmd);
                        goto rmdir;
                }
+               if (WIFEXITED(rslt) && WEXITSTATUS(rslt)) {
+                       rslt = -1;
+                       goto rmdir;
+               }
                if (mount(MOUNT_MSDOS, dst, 0, &args) == -1) {
                        /* Try newfs'ing it. */
                        rslt = create_filesystem(dl, part);

diff --git a/usr.sbin/installboot/loongson_installboot.c b/usr.sbin/installboot/loongson_installboot.c
index 639cc10..e4ddb6c 100644 (file)
--- a/usr.sbin/installboot/loongson_installboot.c
+++ b/usr.sbin/installboot/loongson_installboot.c
@@ -1,4 +1,4 @@
-/*     $OpenBSD: loongson_installboot.c,v 1.4 2021/07/20 14:51:56 kettenis Exp $       */
+/*     $OpenBSD: loongson_installboot.c,v 1.5 2022/09/09 15:53:16 kn Exp $     */
 /*     $NetBSD: installboot.c,v 1.5 1995/11/17 23:23:50 gwr Exp $ */
 
 /*
@@ -41,6 +41,7 @@
 #include <sys/ioctl.h>
 #include <sys/mount.h>
 #include <sys/stat.h>
+#include <sys/wait.h>
 
 #include <err.h>
 #include <errno.h>
@@ -89,8 +90,8 @@ md_installboot(int devfd, char *dev)
 
        part = findmbrfat(devfd, &dl);
        if (part != -1) {
-               write_filesystem(&dl, (char)part);
-               return;
+               if (write_filesystem(&dl, (char)part) == -1)
+                       exit(1);
        }
 }
 
@@ -143,6 +144,10 @@ write_filesystem(struct disklabel *dl, char part)
                        warn("system('%s') failed", cmd);
                        goto rmdir;
                }
+               if (WIFEXITED(rslt) && WEXITSTATUS(rslt)) {
+                       rslt = -1;
+                       goto rmdir;
+               }
                if (mount(MOUNT_EXT2FS, dst, 0, &args) == -1) {
                        /* Try newfs'ing it. */
                        rslt = snprintf(cmd, sizeof(cmd), newfsfmt,
@@ -157,6 +162,10 @@ write_filesystem(struct disklabel *dl, char part)
                                warn("system('%s') failed", cmd);
                                goto rmdir;
                        }
+                       if (WIFEXITED(rslt) && WEXITSTATUS(rslt)) {
+                               rslt = -1;
+                               goto rmdir;
+                       }
                        rslt = mount(MOUNT_EXT2FS, dst, 0, &args);
                        if (rslt == -1) {
                                warn("unable to mount ext2fs partition");

diff --git a/usr.sbin/installboot/macppc_installboot.c b/usr.sbin/installboot/macppc_installboot.c
index 0eb3af5..fe5c1c7 100644 (file)
--- a/usr.sbin/installboot/macppc_installboot.c
+++ b/usr.sbin/installboot/macppc_installboot.c
@@ -1,4 +1,4 @@
-/*     $OpenBSD: macppc_installboot.c,v 1.6 2022/09/03 15:46:20 kn Exp $       */
+/*     $OpenBSD: macppc_installboot.c,v 1.7 2022/09/09 15:53:16 kn Exp $       */
 
 /*
  * Copyright (c) 2011 Joel Sing <jsing@openbsd.org>
@@ -40,6 +40,7 @@
 #include <sys/ioctl.h>
 #include <sys/mount.h>
 #include <sys/stat.h>
+#include <sys/wait.h>
 
 #include <err.h>
 #include <errno.h>
@@ -87,8 +88,8 @@ md_prepareboot(int devfd, char *dev)
 
        part = findmbrfat(devfd, &dl);
        if (part != -1) {
-               create_filesystem(&dl, (char)part);
-               return;
+               if (create_filesystem(&dl, (char)part) == -1)
+                       exit(1);
        }
 }
 
@@ -151,6 +152,8 @@ create_filesystem(struct disklabel *dl, char part)
                        warn("system('%s') failed", cmd);
                        return rslt;
                }
+               if (WIFEXITED(rslt) && WEXITSTATUS(rslt))
+                       return -1;
        }
 
        return 0;
@@ -201,6 +204,10 @@ write_filesystem(struct disklabel *dl, char part)
                        warn("system('%s') failed", cmd);
                        goto rmdir;
                }
+               if (WIFEXITED(rslt) && WEXITSTATUS(rslt)) {
+                       rslt = -1;
+                       goto rmdir;
+               }
                if (mount(MOUNT_MSDOS, dst, 0, &args) == -1) {
                        /* Try newfs'ing it. */
                        rslt = create_filesystem(dl, part);

diff --git a/usr.sbin/installboot/octeon_installboot.c b/usr.sbin/installboot/octeon_installboot.c
index 950e5bb..7a4f36a 100644 (file)
--- a/usr.sbin/installboot/octeon_installboot.c
+++ b/usr.sbin/installboot/octeon_installboot.c
@@ -1,4 +1,4 @@
-/*     $OpenBSD: octeon_installboot.c,v 1.5 2022/08/31 20:52:15 krw Exp $      */
+/*     $OpenBSD: octeon_installboot.c,v 1.6 2022/09/09 15:53:16 kn Exp $       */
 
 /*
  * Copyright (c) 2011 Joel Sing <jsing@openbsd.org>
@@ -40,6 +40,7 @@
 #include <sys/ioctl.h>
 #include <sys/mount.h>
 #include <sys/stat.h>
+#include <sys/wait.h>
 
 #include <err.h>
 #include <errno.h>
@@ -85,8 +86,8 @@ md_prepareboot(int devfd, char *dev)
 
        part = findmbrfat(devfd, &dl);
        if (part != -1) {
-               create_filesystem(&dl, (char)part);
-               return;
+               if (create_filesystem(&dl, (char)part) == -1)
+                       exit(1);
        }
 }
 
@@ -149,6 +150,8 @@ create_filesystem(struct disklabel *dl, char part)
                        warn("system('%s') failed", cmd);
                        return rslt;
                }
+               if (WIFEXITED(rslt) && WEXITSTATUS(rslt))
+                       return -1;
        }
 
        return 0;
@@ -202,6 +205,10 @@ write_filesystem(struct disklabel *dl, char part)
                        warn("system('%s') failed", cmd);
                        goto rmdir;
                }
+               if (WIFEXITED(rslt) && WEXITSTATUS(rslt)) {
+                       rslt = -1;
+                       goto rmdir;
+               }
                if (mount(MOUNT_MSDOS, dst, 0, &args) == -1) {
                        /* Try newfs'ing it. */
                        rslt = create_filesystem(dl, part);

diff --git a/usr.sbin/installboot/powerpc64_installboot.c b/usr.sbin/installboot/powerpc64_installboot.c
index 3e83a1b..e56d6ab 100644 (file)
--- a/usr.sbin/installboot/powerpc64_installboot.c
+++ b/usr.sbin/installboot/powerpc64_installboot.c
@@ -1,4 +1,4 @@
-/*     $OpenBSD: powerpc64_installboot.c,v 1.4 2022/08/31 20:52:15 krw Exp $   */
+/*     $OpenBSD: powerpc64_installboot.c,v 1.5 2022/09/09 15:53:16 kn Exp $    */
 
 /*
  * Copyright (c) 2011 Joel Sing <jsing@openbsd.org>
@@ -40,6 +40,7 @@
 #include <sys/ioctl.h>
 #include <sys/mount.h>
 #include <sys/stat.h>
+#include <sys/wait.h>
 
 #include <err.h>
 #include <errno.h>
@@ -87,8 +88,8 @@ md_prepareboot(int devfd, char *dev)
 
        part = findmbrfat(devfd, &dl);
        if (part != -1) {
-               create_filesystem(&dl, (char)part);
-               return;
+               if (create_filesystem(&dl, (char)part) == -1)
+                       exit(1);
        }
 }
 
@@ -156,6 +157,8 @@ create_filesystem(struct disklabel *dl, char part)
                        warn("system('%s') failed", cmd);
                        return rslt;
                }
+               if (WIFEXITED(rslt) && WEXITSTATUS(rslt))
+                       return -1;
        }
 
        return 0;
@@ -210,6 +213,10 @@ write_filesystem(struct disklabel *dl, char part)
                        warn("system('%s') failed", cmd);
                        goto rmdir;
                }
+               if (WIFEXITED(rslt) && WEXITSTATUS(rslt)) {
+                       rslt = -1;
+                       goto rmdir;
+               }
                if (mount(MOUNT_MSDOS, dir, 0, &args) == -1) {
                        /* Try newfs'ing it. */
                        rslt = create_filesystem(dl, part);