From 4f59b3df6fd76b5bfbd8e86d025eab4627391b49 Mon Sep 17 00:00:00 2001 From: Rusty Bird Date: Wed, 18 Nov 2015 13:11:30 +0000 Subject: [PATCH] qfile-unpacker: syncfs() to avoid qvm-move-to-vm data loss Commit https://github.com/QubesOS/qubes-linux-utils/commit/c1d42f1 -- "qfile-unpacker: do not call fdatasync() at each file" fixing QubesOS/qubes-issues#1257 -- increased the chance of data loss with qvm-move-to-vm: Say it nominally succeeds, and *deletes* the files from the source VM. Soon after, the destination VM or the system could crash, or an external drive hosting ~/QubesIncoming/srcVM could get unplugged by accident, all before the data had really been persisted to disk. But reverting the commit (ignoring the performance issue) wouldn't completely solve this: "Calling fsync() does not necessarily ensure that the entry in the directory containing the file has also reached disk. For that an explicit fsync() on a file descriptor for the directory is also needed." - fsync(2) It gets even worse for "slow symlinks" (whose target is too long to be stored directly in the inode metadata), apparently they can't be synced at all individually. So instead, just call syncfs() once after everything has been unpacked: + Should prevent all data loss (if fs and disk are well behaved) + Allows caching and reordering -> no slowdown with many small files - Blocks until any unrelated writes on the filesystem finish :\ --- qrexec-lib/unpack.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/qrexec-lib/unpack.c b/qrexec-lib/unpack.c index a76a934..fbb0b89 100644 --- a/qrexec-lib/unpack.c +++ b/qrexec-lib/unpack.c @@ -208,6 +208,9 @@ void process_one_file(struct file_header *untrusted_hdr) int do_unpack(void) { struct file_header untrusted_hdr; + int cwd_fd; + int saved_errno; + total_bytes = total_files = 0; /* initialize checksum */ crc32_sum = 0; @@ -222,6 +225,12 @@ int do_unpack(void) do_exit(EDQUOT, untrusted_namebuf); process_one_file(&untrusted_hdr); } + + saved_errno = errno; + cwd_fd = open(".", O_RDONLY); + if (cwd_fd >= 0 && syncfs(cwd_fd) == 0 && close(cwd_fd) == 0) + errno = saved_errno; + send_status_and_crc(errno, untrusted_namebuf); return errno; }