From: Jan Kara <jack@suse.cz>
To: Christian Brauner <brauner@kernel.org>
Cc: <linux-fsdevel@vger.kernel.org>,
Amir Goldstein <amir73il@gmail.com>,
James Clark <james.clark@arm.com>,
linux-nfs@vger.kernel.org, NeilBrown <neilb@suse.de>,
Al Viro <viro@ZenIV.linux.org.uk>,
ltp@lists.linux.it, Jan Kara <jack@suse.cz>
Subject: [PATCH 2/2] vfs: generate FS_CREATE before FS_OPEN when ->atomic_open used.
Date: Mon, 17 Jun 2024 18:23:01 +0200 [thread overview]
Message-ID: <20240617162303.1596-2-jack@suse.cz> (raw)
In-Reply-To: <20240617161828.6718-1-jack@suse.cz>
From: NeilBrown <neilb@suse.de>
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 open_last_lookups() 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@arm.com>
Closes: https://lore.kernel.org/all/01c3bf2e-eb1f-4b7f-a54f-d2a05dd3d8c8@arm.com
Signed-off-by: NeilBrown <neilb@suse.de>
Link: https://lore.kernel.org/r/171817619547.14261.975798725161704336@noble.neil.brown.name
Fixes: 7b8c9d7bb457 ("fsnotify: move fsnotify_open() hook into do_dentry_open()")
Tested-by: James Clark <james.clark@arm.com>
Signed-off-by: Christian Brauner <brauner@kernel.org>
Signed-off-by: Jan Kara <jack@suse.cz>
---
fs/namei.c | 10 ++++++++--
fs/open.c | 22 +++++++++++++++-------
2 files changed, 23 insertions(+), 9 deletions(-)
diff --git a/fs/namei.c b/fs/namei.c
index 37fb0a8aa09a..1e05a0f3f04d 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -3572,8 +3572,12 @@ static const char *open_last_lookups(struct nameidata *nd,
else
inode_lock_shared(dir->d_inode);
dentry = lookup_open(nd, file, op, got_write);
- if (!IS_ERR(dentry) && (file->f_mode & FMODE_CREATED))
- fsnotify_create(dir->d_inode, dentry);
+ if (!IS_ERR(dentry)) {
+ if (file->f_mode & FMODE_CREATED)
+ fsnotify_create(dir->d_inode, dentry);
+ if (file->f_mode & FMODE_OPENED)
+ fsnotify_open(file);
+ }
if (open_flag & O_CREAT)
inode_unlock(dir->d_inode);
else
@@ -3700,6 +3704,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..f1607729acb9 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,19 @@ 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,
@@ -1177,8 +1183,10 @@ struct file *kernel_file_open(const struct path *path, int flags,
error = do_dentry_open(f, NULL);
if (error) {
fput(f);
- f = ERR_PTR(error);
+ return ERR_PTR(error);
}
+
+ fsnotify_open(f);
return f;
}
EXPORT_SYMBOL_GPL(kernel_file_open);
--
2.35.3
next prev parent reply other threads:[~2024-06-17 16:23 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-06-17 16:22 [PATCH 0/2 v3] fsnotify: Generate FS_CREATE event before FS_OPEN Jan Kara
2024-06-17 16:23 ` [PATCH 1/2] fsnotify: Do not generate events for O_PATH file descriptors Jan Kara
2024-06-17 17:10 ` Amir Goldstein
2024-06-18 14:28 ` Christian Brauner
2024-06-19 15:24 ` Jan Kara
2024-06-17 16:23 ` Jan Kara [this message]
2024-06-17 17:12 ` [PATCH 2/2] vfs: generate FS_CREATE before FS_OPEN when ->atomic_open used Amir Goldstein
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20240617162303.1596-2-jack@suse.cz \
--to=jack@suse.cz \
--cc=amir73il@gmail.com \
--cc=brauner@kernel.org \
--cc=james.clark@arm.com \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-nfs@vger.kernel.org \
--cc=ltp@lists.linux.it \
--cc=neilb@suse.de \
--cc=viro@ZenIV.linux.org.uk \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox