[PATCH v2] VFS: generate FS_CREATE before FS_OPEN when ->atomic_open used.

From: NeilBrown
Date: Wed Jun 12 2024 - 03:10:39 EST



When a file is opened and created with open(..., O_CREAT) we get
both the CREATE and OPEN fsnotify events and would expect them in that
order. For most filesystems we get them in that order because
open_last_lookups() calls fsnofify_create() and then do_open() (from
path_openat()) calls vfs_open()->do_dentry_open() which calls
fsnotify_open().

However when ->atomic_open is used, the
do_dentry_open() -> fsnotify_open()
call happens from finish_open() which is called from the ->atomic_open
handler in lookup_open() which is called *before* open_last_lookups()
calls fsnotify_create. So we get the "open" notification before
"create" - which is backwards. ltp testcase inotify02 tests this and
reports the inconsistency.

This patch lifts the fsnotify_open() call out of do_dentry_open() and
places it higher up the call stack. There are three callers of
do_dentry_open().

For vfs_open() and kernel_file_open() the fsnotify_open() is placed
directly in that caller so there should be no behavioural change.

For finish_open() there are two cases:
- finish_open is used in ->atomic_open handlers. For these we add a
call to fsnotify_open() at the top of do_open() if FMODE_OPENED is
set - which means do_dentry_open() has been called.
- finish_open is used in ->tmpfile() handlers. For these a similar
call to fsnotify_open() is added to vfs_tmpfile()

With this patch NFSv3 is restored to its previous behaviour (before
->atomic_open support was added) of generating CREATE notifications
before OPEN, and NFSv4 now has that same correct ordering that is has
not had before. I haven't tested other filesystems.

Fixes: 7c6c5249f061 ("NFS: add atomic_open for NFSv3 to handle O_TRUNC correctly.")
Reported-by: James Clark <james.clark@xxxxxxx>
Closes: https://lore.kernel.org/all/01c3bf2e-eb1f-4b7f-a54f-d2a05dd3d8c8@xxxxxxx
Signed-off-by: NeilBrown <neilb@xxxxxxx>
---
fs/namei.c | 5 +++++
fs/open.c | 19 ++++++++++++-------
2 files changed, 17 insertions(+), 7 deletions(-)

diff --git a/fs/namei.c b/fs/namei.c
index 37fb0a8aa09a..057afacc4b60 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -3612,6 +3612,9 @@ static int do_open(struct nameidata *nd,
int acc_mode;
int error;

+ if (file->f_mode & FMODE_OPENED)
+ fsnotify_open(file);
+
if (!(file->f_mode & (FMODE_OPENED | FMODE_CREATED))) {
error = complete_walk(nd);
if (error)
@@ -3700,6 +3703,8 @@ int vfs_tmpfile(struct mnt_idmap *idmap,
mode = vfs_prepare_mode(idmap, dir, mode, mode, mode);
error = dir->i_op->tmpfile(idmap, dir, file, mode);
dput(child);
+ if (file->f_mode & FMODE_OPENED)
+ fsnotify_open(file);
if (error)
return error;
/* Don't check for other permissions, the inode was just created */
diff --git a/fs/open.c b/fs/open.c
index 89cafb572061..970f299c0e77 100644
--- a/fs/open.c
+++ b/fs/open.c
@@ -1004,11 +1004,6 @@ static int do_dentry_open(struct file *f,
}
}

- /*
- * Once we return a file with FMODE_OPENED, __fput() will call
- * fsnotify_close(), so we need fsnotify_open() here for symmetry.
- */
- fsnotify_open(f);
return 0;

cleanup_all:
@@ -1085,8 +1080,17 @@ EXPORT_SYMBOL(file_path);
*/
int vfs_open(const struct path *path, struct file *file)
{
+ int ret;
+
file->f_path = *path;
- return do_dentry_open(file, NULL);
+ ret = do_dentry_open(file, NULL);
+ if (!ret)
+ /*
+ * Once we return a file with FMODE_OPENED, __fput() will call
+ * fsnotify_close(), so we need fsnotify_open() here for symmetry.
+ */
+ fsnotify_open(file);
+ return ret;
}

struct file *dentry_open(const struct path *path, int flags,
@@ -1178,7 +1182,8 @@ struct file *kernel_file_open(const struct path *path, int flags,
if (error) {
fput(f);
f = ERR_PTR(error);
- }
+ } else
+ fsnotify_open(f);
return f;
}
EXPORT_SYMBOL_GPL(kernel_file_open);
--
2.44.0