From a88eaab9c8eefa2a431a8777589db40db4bb0c09 Mon Sep 17 00:00:00 2001
From: Daniel Carl Jones <djonesoa@amazon.com>
Date: Mon, 30 Sep 2024 21:57:57 +0100
Subject: [PATCH] Update O_SYNC/O_DSYNC open flag check to occur ahead of
 lookup

Signed-off-by: Daniel Carl Jones <djonesoa@amazon.com>
---
 mountpoint-s3/src/fs.rs   | 12 ++++++------
 mountpoint-s3/tests/fs.rs | 23 +++++++++++++++++++++++
 2 files changed, 29 insertions(+), 6 deletions(-)

diff --git a/mountpoint-s3/src/fs.rs b/mountpoint-s3/src/fs.rs
index b65762a3c..efe2f9cb7 100644
--- a/mountpoint-s3/src/fs.rs
+++ b/mountpoint-s3/src/fs.rs
@@ -767,6 +767,12 @@ where
         #[cfg(target_os = "linux")]
         let direct_io = flags & libc::O_DIRECT != 0;
 
+        // We can't support O_SYNC writes because they require the data to go to stable storage
+        // at `write` time, but we only commit a PUT at `close` time.
+        if flags & (libc::O_SYNC | libc::O_DSYNC) != 0 {
+            return Err(err!(libc::EINVAL, "O_SYNC and O_DSYNC are not supported"));
+        }
+
         let force_revalidate = !self.config.cache_config.serve_lookup_from_cache || direct_io;
         let lookup = self.superblock.getattr(&self.client, ino, force_revalidate).await?;
 
@@ -785,12 +791,6 @@ where
             return Err(err!(libc::EINVAL, "O_APPEND is not supported on existing files"));
         }
 
-        // We can't support O_SYNC writes because they require the data to go to stable storage
-        // at `write` time, but we only commit a PUT at `close` time.
-        if flags & (libc::O_SYNC | libc::O_DSYNC) != 0 {
-            return Err(err!(libc::EINVAL, "O_SYNC and O_DSYNC are not supported"));
-        }
-
         let state = if flags & libc::O_RDWR != 0 {
             let is_truncate = flags & libc::O_TRUNC != 0;
             if !remote_file || (self.config.allow_overwrite && is_truncate) {
diff --git a/mountpoint-s3/tests/fs.rs b/mountpoint-s3/tests/fs.rs
index 3eeb7eea0..632733871 100644
--- a/mountpoint-s3/tests/fs.rs
+++ b/mountpoint-s3/tests/fs.rs
@@ -700,6 +700,29 @@ async fn test_unordered_write_fails(offset: i64) {
     assert_eq!(err, libc::EINVAL);
 }
 
+#[test_case(libc::O_SYNC; "O_SYNC")]
+#[test_case(libc::O_DSYNC; "O_DSYNC")]
+#[tokio::test]
+async fn test_sync_flags_fail(flag: libc::c_int) {
+    const BUCKET_NAME: &str = "test_sync_flags_fail";
+
+    let (_client, fs) = make_test_filesystem(BUCKET_NAME, &Default::default(), Default::default());
+
+    let mode = libc::S_IFREG | libc::S_IRWXU; // regular file + 0700 permissions
+    let dentry = fs
+        .mknod(FUSE_ROOT_INODE, "file2.bin".as_ref(), mode, 0, 0)
+        .await
+        .unwrap();
+    let file_ino = dentry.attr.ino;
+
+    let err = fs
+        .open(file_ino, libc::S_IFREG as i32 | flag, 0)
+        .await
+        .expect_err("open should fail due to use of O_SYNC/O_DSYNC flag")
+        .to_errno();
+    assert_eq!(err, libc::EINVAL);
+}
+
 #[tokio::test]
 async fn test_duplicate_write_fails() {
     const BUCKET_NAME: &str = "test_duplicate_write_fails";