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 :\
pull/2/head
Rusty Bird 9 years ago
parent 8311e1263d
commit 4f59b3df6f
No known key found for this signature in database
GPG Key ID: 469D78F47AAF2ADF

@ -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;
}

Loading…
Cancel
Save