git: 87e63f2e7f92 - main - nextboot: Write nextboot.conf safely

From: Mark Johnston <markj_at_FreeBSD.org>
Date: Wed, 03 Apr 2024 15:31:53 UTC
The branch main has been updated by markj:

URL: https://cgit.FreeBSD.org/src/commit/?id=87e63f2e7f92e192c1e8b01ac7cc45b88c03a478

commit 87e63f2e7f92e192c1e8b01ac7cc45b88c03a478
Author:     Mark Johnston <markj@FreeBSD.org>
AuthorDate: 2024-04-03 15:29:25 +0000
Commit:     Mark Johnston <markj@FreeBSD.org>
CommitDate: 2024-04-03 15:31:43 +0000

    nextboot: Write nextboot.conf safely
    
    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.
    
    Fixes:  fd6d47375a78 ("rescue,nextboot: Install nextboot as a link to reboot, rm nextboot.sh")
    Reviewed by:    imp
    Differential Revision:  https://reviews.freebsd.org/D44572
---
 sbin/reboot/reboot.c | 38 +++++++++++++++++++++++++++++---------
 1 file changed, 29 insertions(+), 9 deletions(-)

diff --git a/sbin/reboot/reboot.c b/sbin/reboot/reboot.c
index 3cc77aebe4d4..1b806a90de1d 100644
--- a/sbin/reboot/reboot.c
+++ b/sbin/reboot/reboot.c
@@ -113,8 +113,10 @@ zfsbootcfg(const char *pool, bool force)
 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;
 
@@ -138,21 +140,39 @@ write_nextboot(const char *fn, const char *env, bool force)
 		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", 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 write %s", fn);
+		E("Can't rename %s to %s", tmp, fn);
 	}
 	fclose(fp);
 }
@@ -194,8 +214,8 @@ add_env(char **env, const char *key, const char *value)
 /*
  * 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[])