nextboot: Write nextboot.conf safely

[markj]

Summary

As in the old nextboot.sh script:

• First write everything to a tempfile instead of /boot/nextboot.conf.
• fsync() the tempfile before renaming it to nextboot.conf.

While here fix handling of -f in getopt().

Fixes: fd6d47375a78 ("rescue,nextboot: Install nextboot as a link to reboot, rm nextboot.sh")

[imp]

I don’t think the raw err calls are desirable for the force case.

sbin/reboot/reboot.c

153

This is a change in behavior (here and elsewhere)... any errors in the old script in crwating nextboot mean we'd not fail to reboot anyway...

It may be more correct, but now will result in different behavior for, say, readonly /. I was torn on it, but didn't want to break automated scripting that depends on it since we'd fail with the system still running when we were forcing a reboot even if we couldn't write nextboot...

[markj]

sbin/reboot/reboot.c

153

Ah, I forgot that this code is used not just by nextboot(8).

But, -f is already broken: neither the reboot(8) nor the nextboot(8) getopt strings include 'f', so force is always false. The documentation for -f in both nextboot.8 and reboot.8 also says that it's only used to force reboot to succeed if the kernel doesn't exist, not for other reasons.

[markj]

• Restore use of E().
• Make nextboot/reboot handle -f.

[imp]

Ok. We should put this in the relnotes then. Uggg I made a mess of things trying to sinplify :(

[markj]

In D44572#1016397, @imp wrote:

Ok. We should put this in the relnotes then. Uggg I made a mess of things trying to sinplify :(

Put what in the relnotes exactly? I think the simplification was fine, we just have some regressions, none of which are in 14.0.

sbin/reboot/reboot.c

zfsbootcfg(const char *pool, bool force)
         if (WIFSIGNALED(status))
                 E("zfsbootcfg died with signal %d", WTERMSIG(status));
         E("zfsbootcfg unexpected status %d", status);
 }
 
 static void
 write_nextboot(const char *fn, const char *env, bool force)
 {
+        char tmp[PATH_MAX];
         FILE *fp;
         struct statfs sfs;
+        int tmpfd;
         bool supported = false;
         bool zfs = false;
 
         if (statfs("/boot", &sfs) != 0)
                 err(1, "statfs /boot");
         if (strcmp(sfs.f_fstypename, "ufs") == 0) {
                 /*
                  * Only UFS supports the full nextboot protocol.
                  */
                 supported = true;
         } else if (strcmp(sfs.f_fstypename, "zfs") == 0) {
                 zfs = true;
         }
 
         if (zfs) {
                 char *slash;
 
                 if ((slash = strchr(sfs.f_mntfromname, '/')) == NULL)
                         E("Can't find ZFS pool name in %s", sfs.f_mntfromname);
                 *slash = '\0';
                 zfsbootcfg(sfs.f_mntfromname, force);
         }
 
-        fp = fopen(fn, "w");
+        if (strlcpy(tmp, fn, sizeof(tmp)) >= sizeof(tmp))
+                E("Path too long %s", fn);
+        if (strlcat(tmp, ".XXXXXX", sizeof(tmp)) >= sizeof(tmp))
+                E("Path too long %s", fn);
+        tmpfd = mkstemp(tmp);
+        if (tmpfd == -1)
+                E("mkstemp %s", tmp);
+
+        fp = fdopen(tmpfd, "w");
         if (fp == NULL)
-                E("Can't create %s", fn);
+                E("fdopen %s", tmp);
 
-        if (fprintf(fp,"%s%s",
+        if (fprintf(fp, "%s%s",
             supported ? "nextboot_enable=\"YES\"\n" : "",
             env != NULL ? env : "") < 0) {
                 int e;
 
                 e = errno;
-                fclose(fp);
-                if (unlink(fn))
-                        warn("unlink %s", fn);
+                if (unlink(tmp))
+                        warn("unlink %s", tmp);
                 errno = e;
-                E("Can't write %s", fn);
+                E("Can't write %s", tmp);
         }
+        if (fsync(fileno(fp)) != 0)
+                E("Can't fsync %s", fn);
+        if (rename(tmp, fn) != 0) {
+                int e;
+
+                e = errno;
+                if (unlink(tmp))
+                        warn("unlink %s", tmp);
+                errno = e;
+                E("Can't rename %s to %s", tmp, fn);
+        }
         fclose(fp);
 }
 
 static char *
 split_kv(char *raw)
 {
         char *eq;
         int len;
add_env(char **env, const char *key, const char *value)
         if (env == NULL)
                 errx(1, "No memory to build env array");
         free(oldenv);
 }
 
 /*
  * Different options are valid for different programs.
  */
-#define GETOPT_REBOOT "cDde:k:lNno:pqr"
-#define GETOPT_NEXTBOOT "De:k:o:"
+#define GETOPT_REBOOT "cDde:fk:lNno:pqr"
+#define GETOPT_NEXTBOOT "De:fk:o:"
 
 int
 main(int argc, char *argv[])
 {
         struct utmpx utx;
         const struct passwd *pw;
         int ch, howto = 0, i, sverrno;
         bool Dflag, fflag, lflag, Nflag, nflag, qflag;