From 99127edf80ca0c3e034474ab24bf3c4a9690acdf Mon Sep 17 00:00:00 2001
From: Michalis Kokologiannakis <mixaskok@gmail.com>
Date: Mon, 29 Jun 2020 13:43:41 +0300
Subject: [PATCH] files: improve backup procedure to ensure no data is lost

The current save procedure followed by nano is not crash-safe
under ext4 (and perhaps other filesystems) when editing existing
files. Specifically, modifying an existing file can lead to data
loss, since a modified file is not replaced atomically on disk.
Using "-B" does not ensure crash-safety since the backup may
not have persisted on disk when the writeout of the file starts.

In addition, if I/O pressure fills up the remaining disk space
after an existing file is truncated during save, nano will not
be able to finish saving the file, which will remain truncated.

This commit address these issues by making nano do the following:

1) Take a temporary backup of the file being written so that
there is no time window such that (a) an existing file is truncated
and (b) the buffer corresponding to said file has not been
flushed to disk. Such windows are dangerous because, under certain
circumstances, they can lead to data loss under ext4.

The backup is taken either by renaming the original file, or,
in case renaming fails, by copying it.

2) Use fsync() so that, when save finishes, it is certain that
the file has been successfully written to disk.

Signed-off-by: Michalis Kokologiannakis <michalis@mpi-sws.org>
---
 src/files.c | 168 ++++++++++++++++++++++++++++++++++------------------
 1 file changed, 110 insertions(+), 58 deletions(-)

diff --git a/src/files.c b/src/files.c
index a110567e..933507a7 100644
--- a/src/files.c
+++ b/src/files.c
@@ -1571,6 +1571,24 @@ int copy_file(FILE *inn, FILE *out, bool close_out)
 	return retval;
 }
 
+/* Syncs out the contents of a given file to disk.
+ * Returns 0 on success, and a negative number on failure.
+ * In case of failure, the file is closed */
+int sync_file(FILE *thefile)
+{
+	/* Send all buffered data from fwrite()s to the kernel. */
+	if (fflush(thefile) != 0) {
+		fclose(thefile);
+		return -1;
+	}
+	/* Make sure data reaches disk. */
+	if (fsync(fileno(thefile)) != 0) {
+		fclose(thefile);
+		return -2;
+	}
+	return 0;
+}
+
 /* Write the current buffer to disk.  If thefile isn't NULL, we write to a
  * temporary file that is already open.  If tmp is TRUE (when spell checking
  * or emergency dumping, for example), we set the umask to disallow anyone else
@@ -1589,6 +1607,10 @@ bool write_file(const char *name, FILE *thefile, bool tmp,
 		/* The status fields filled in by stat(). */
 	char *backupname = NULL;
 		/* The name of the backup file, in case we make one. */
+	bool backup_by_move = FALSE;
+		/* Whether the backup was created by moving the original. */
+	bool is_failsafe_backup = FALSE;
+		/* Whether normal backup failed and we are restorting to a failsafe. */
 #endif
 	char *realname = real_dir_from_tilde(name);
 		/* The filename after tilde expansion. */
@@ -1622,31 +1644,15 @@ bool write_file(const char *name, FILE *thefile, bool tmp,
 	/* When the user requested a backup, we do this only if the file exists and
 	 * isn't temporary AND the file has not been modified by someone else since
 	 * we opened it (or we are appending/prepending or writing a selection). */
-	if (ISSET(MAKE_BACKUP) && is_existing_file && openfile->statinfo &&
+	if (is_existing_file && openfile->statinfo &&
 						(openfile->statinfo->st_mtime == st.st_mtime ||
 						method != OVERWRITE || openfile->mark)) {
-		static struct timespec filetime[2];
-		int backup_cflags, backup_fd, verdict;
-		FILE *original = NULL, *backup_file = NULL;
-
-		/* Remember the original file's access and modification times. */
-		filetime[0].tv_sec = openfile->statinfo->st_atime;
-		filetime[1].tv_sec = openfile->statinfo->st_mtime;
-
-		/* Open the file of which a backup must be made. */
-		original = fopen(realname, "rb");
 
-		/* If we can't read from the original file, go on, since saving
-		 * only the current buffer is better than saving nothing. */
-		if (original == NULL) {
-			statusline(ALERT, _("Error reading %s: %s"), realname, strerror(errno));
-			goto skip_backup;
-		}
-
-		/* If no backup directory was specified, we make a simple backup
-		 * by appending a tilde to the original file name.  Otherwise,
-		 * we create a numbered backup in the specified directory. */
-		if (backup_dir == NULL) {
+		/* If no backup directory was specified or this is a temporary
+		 * backup, we make a simple backup by appending a tilde to the
+		 * original file name.  Otherwise, we create a numbered backup
+		 * in the specified directory. */
+		if (backup_dir == NULL || !ISSET(MAKE_BACKUP)) {
 			backupname = charalloc(strlen(realname) + 2);
 			sprintf(backupname, "%s~", realname);
 		} else {
@@ -1674,22 +1680,41 @@ bool write_file(const char *name, FILE *thefile, bool tmp,
 			 * be fond of backups.  Thus, without one, do not go on. */
 			if (*backupname == '\0') {
 				statusline(ALERT, _("Too many existing backup files"));
-				fclose(original);
 				goto cleanup_and_exit;
 			}
 		}
 
-		/* Now first try to delete an existing backup file. */
-		if (unlink(backupname) < 0 && errno != ENOENT && !ISSET(INSECURE_BACKUP)) {
-			warn_and_briefly_pause(_("Cannot delete existing backup"));
-			fclose(original);
-			if (user_wants_to_proceed())
-				goto skip_backup;
-			statusline(HUSH, _("Cannot delete backup %s: %s"),
-								backupname, strerror(errno));
-			goto cleanup_and_exit;
+		/* Fastpath: Try to move the existing file to create the backup. */
+		if (rename(realname, backupname) == 0) {
+			backup_by_move = TRUE;
+			goto backup_done;
+		}
+
+		/* Slowpath: Moving failed; maybe the sticky bit on the backup's
+		 * directory is set. Try to copy the file instead. */
+		static struct timespec filetime[2];
+		int backup_cflags, backup_fd, verdict;
+		FILE *original = NULL, *backup_file = NULL;
+
+		/* Remember the original file's access and modification times. */
+		filetime[0].tv_sec = openfile->statinfo->st_atime;
+		filetime[1].tv_sec = openfile->statinfo->st_mtime;
+
+		/* Open the file of which a backup must be made. */
+		original = fopen(realname, "rb");
+
+		/* If we can't read from the original file, go on, since saving
+		 * only the current buffer is better than saving nothing. */
+		if (original == NULL) {
+			statusline(ALERT, _("Error reading %s: %s"), realname, strerror(errno));
+			goto backup_done;
 		}
 
+  try_backup:
+		/* Now first try to delete an existing backup file. */
+		if (unlink(backupname) < 0 && errno != ENOENT && !ISSET(INSECURE_BACKUP))
+			goto backup_error;
+
 		if (ISSET(INSECURE_BACKUP))
 			backup_cflags = O_WRONLY | O_CREAT | O_TRUNC;
 		else
@@ -1701,15 +1726,8 @@ bool write_file(const char *name, FILE *thefile, bool tmp,
 		if (backup_fd >= 0)
 			backup_file = fdopen(backup_fd, "wb");
 
-		if (backup_file == NULL) {
-			warn_and_briefly_pause(_("Cannot create backup file"));
-			fclose(original);
-			if (user_wants_to_proceed())
-				goto skip_backup;
-			statusline(HUSH, _("Cannot create backup %s: %s"),
-								backupname, strerror(errno));
-			goto cleanup_and_exit;
-		}
+		if (backup_file == NULL)
+			goto backup_error;
 
 		/* Try to change owner and group to those of the original file;
 		 * ignore errors, as a normal user cannot change the owner. */
@@ -1724,38 +1742,57 @@ bool write_file(const char *name, FILE *thefile, bool tmp,
 		/* Copy the existing file to the backup. */
 		verdict = copy_file(original, backup_file, FALSE);
 
+		/* Since the backup is a newly created file, explicitly fsync() it
+		 * to make sure that it has persisted to permanent storage before
+		 * the start of the write out of the original file */
+		if (sync_file(backup_file) != 0)
+			goto backup_error;
+
 		if (verdict < 0) {
 			fclose(backup_file);
 			statusline(ALERT, _("Error reading %s: %s"), realname, strerror(errno));
 			goto cleanup_and_exit;
 		} else if (verdict > 0) {
 			fclose(backup_file);
-			warn_and_briefly_pause(_("Cannot write backup file"));
-			if (user_wants_to_proceed())
-				goto skip_backup;
-			statusline(HUSH, _("Cannot write backup %s: %s"),
-								backupname, strerror(errno));
-			goto cleanup_and_exit;
+			goto backup_error;
 		}
 
 		/* Set the backup's timestamps to those of the original file.
 		 * Failure is unimportant: saving the file apparently worked. */
 		IGNORE_CALL_RESULT(futimens(backup_fd, filetime));
 
-		if (fclose(backup_file) != 0) {
-			warn_and_briefly_pause(_("Cannot write backup"));
-			if (user_wants_to_proceed())
-				goto skip_backup;
-			statusline(HUSH, _("Cannot write backup %s: %s"),
-								backupname, strerror(errno));
-			goto cleanup_and_exit;
+		if (fclose(backup_file) == 0)
+			goto backup_done;
+
+  backup_error:
+		/* If the first attempt of backing up the file failed, try again at HOME. */
+		if (!is_failsafe_backup) {
+			unlink(backupname);
+			free(backupname);
+			backupname = charalloc(strlen(homedir) + strlen(tail(realname)) + 2);
+			sprintf(backupname, "%s/%s~", homedir, tail(realname));
+			is_failsafe_backup = TRUE;
+			goto try_backup;
+		}
+
+		/* If all attempts failed, notify user. If something goes wrong
+		 * during save the contents of the file might be lost. */
+		warn_and_briefly_pause(_("Cannot create backup"));
+		fclose(original);
+		if (user_wants_to_proceed()) {
+			goto backup_done;
 		}
+		statusline(HUSH, _("Cannot create backup %s: %s"),
+			   backupname, strerror(errno));
+		goto cleanup_and_exit;
+
 	}
 
-  skip_backup:
+  backup_done:
 	/* When prepending, first copy the existing file to a temporary file. */
 	if (method == PREPEND) {
-		FILE *source = fopen(realname, "rb");
+		char *original = (backup_by_move) ? backupname : realname;
+		FILE *source = fopen(original, "rb");
 		FILE *target = NULL;
 		int verdict;
 
@@ -1894,24 +1931,39 @@ bool write_file(const char *name, FILE *thefile, bool tmp,
 			goto cleanup_and_exit;
 		}
 
-		verdict = copy_file(source, thefile, TRUE);
+		verdict = copy_file(source, thefile, FALSE);
 
 		if (verdict < 0) {
+			fclose(thefile);
 			statusline(ALERT, _("Error reading temp file: %s"), strerror(errno));
 			goto cleanup_and_exit;
 		} else if (verdict > 0) {
+			fclose(thefile);
 			statusline(ALERT, _("Error writing %s: %s"), realname, strerror(errno));
 			goto cleanup_and_exit;
 		}
 
 		unlink(tempname);
-	} else
+	}
 #endif
+	if (sync_file(thefile) != 0) {
+		statusline(ALERT, _("Error writing %s: %s"), realname, strerror(errno));
+		goto cleanup_and_exit;
+	}
+
 	if (fclose(thefile) != 0) {
 		statusline(ALERT, _("Error writing %s: %s"), realname, strerror(errno));
 		goto cleanup_and_exit;
 	}
 
+	/* If the user hasn't specified that a backup should be created, delete the
+	 * temporary backup created to ensure the atomic replacement of the file. */
+	if (!ISSET(MAKE_BACKUP) && backupname != NULL) {
+		/* Doesn't really matter if this fails, since the file has been written. */
+		if (unlink(backupname) != 0)
+			warn_and_briefly_pause(_("Cannot delete temporary backup"));
+	}
+
 	/* When having written an entire buffer, update some administrivia. */
 	if (fullbuffer && method == OVERWRITE && !tmp) {
 		/* If the filename was changed, write a new lockfile when needed,
-- 
2.27.0

