From 12cc4e5f5bbe8da2b59e10fef05b9d30156fc39e Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Fri, 30 Apr 2021 12:44:30 -0400 Subject: [PATCH] composepost: Fix race condition in timestamp checking `syncfs()` isn't going to do anything on e.g. `tmpfs` and even if it did wouldn't fix any race conditions because that's about synchronizing in memory changes to disk, but won't change what system calls return. Some investigation turned up https://stackoverflow.com/questions/14392975/timestamp-accuracy-on-ext4-sub-millsecond and `current_fs_time` is now: https://www.kernel.org/doc/html/v5.12/core-api/timekeeping.html Basically there's a "coarse" time that might only update once every 10ms for example. Let's just sleep 100ms for now. I think we should be using the inode versions, but we can investigate that separately. --- rust/src/composepost.rs | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/rust/src/composepost.rs b/rust/src/composepost.rs index 04c16bee..f5ef1dff 100644 --- a/rust/src/composepost.rs +++ b/rust/src/composepost.rs @@ -1171,25 +1171,21 @@ OSTREE_VERSION='33.4' rootfs.new_file(&binpath, 0o755).unwrap(); let fpath = format!("{}/{}", PREFIX, fname); rootfs.new_file(&fpath, 0o755).unwrap(); - rootfs.syncfs().unwrap(); let before_meta = rootfs.metadata(fpath).unwrap(); metas.push(before_meta); } + // File timestamps may not increment faster than e.g. 10ms. Really + // what we should be doing is using inode versions. + std::thread::sleep(std::time::Duration::from_millis(100)); tweak_selinux_timestamps(&rootfs, gio::NONE_CANCELLABLE).unwrap(); - rootfs.syncfs().unwrap(); for (fname, before_meta) in files.iter().zip(metas.iter()) { let fpath = format!("{}/{}", PREFIX, fname); let after = rootfs.metadata(&fpath).unwrap(); - assert!( - before_meta.stat().st_atime != after.stat().st_atime - || before_meta.stat().st_atime_nsec != after.stat().st_atime_nsec - ); - assert!( - before_meta.stat().st_mtime != after.stat().st_mtime - || before_meta.stat().st_mtime_nsec != after.stat().st_mtime_nsec - ); + if before_meta.stat().st_mtime == after.stat().st_mtime { + assert_ne!(before_meta.stat().st_mtime_nsec, after.stat().st_mtime_nsec); + } } }