diff mbox series

[1/2] fsnotify: Do not generate events for O_PATH file descriptors

Message ID 20240617162303.1596-1-jack@suse.cz
State Handled Elsewhere
Headers show
Series fsnotify: Generate FS_CREATE event before FS_OPEN | expand

Commit Message

Jan Kara June 17, 2024, 4:23 p.m. UTC
Currently we will not generate FS_OPEN events for O_PATH file
descriptors but we will generate FS_CLOSE events for them. This is
asymmetry is confusing. Arguably no fsnotify events should be generated
for O_PATH file descriptors as they cannot be used to access or modify
file content, they are just convenient handles to file objects like
paths. So fix the asymmetry by stopping to generate FS_CLOSE for O_PATH
file descriptors.

Signed-off-by: Jan Kara <jack@suse.cz>
---
 include/linux/fsnotify.h | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

Comments

Amir Goldstein June 17, 2024, 5:10 p.m. UTC | #1
On Mon, Jun 17, 2024 at 7:23 PM Jan Kara <jack@suse.cz> wrote:
>
> Currently we will not generate FS_OPEN events for O_PATH file
> descriptors but we will generate FS_CLOSE events for them. This is
> asymmetry is confusing. Arguably no fsnotify events should be generated
> for O_PATH file descriptors as they cannot be used to access or modify
> file content, they are just convenient handles to file objects like
> paths. So fix the asymmetry by stopping to generate FS_CLOSE for O_PATH
> file descriptors.
>
> Signed-off-by: Jan Kara <jack@suse.cz>

Looks good.

Reviewed-by: Amir Goldstein <amir73il@gmail.com>

> ---
>  include/linux/fsnotify.h | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/include/linux/fsnotify.h b/include/linux/fsnotify.h
> index 4da80e92f804..278620e063ab 100644
> --- a/include/linux/fsnotify.h
> +++ b/include/linux/fsnotify.h
> @@ -112,7 +112,13 @@ static inline int fsnotify_file(struct file *file, __u32 mask)
>  {
>         const struct path *path;
>
> -       if (file->f_mode & FMODE_NONOTIFY)
> +       /*
> +        * FMODE_NONOTIFY are fds generated by fanotify itself which should not
> +        * generate new events. We also don't want to generate events for
> +        * FMODE_PATH fds (involves open & close events) as they are just
> +        * handle creation / destruction events and not "real" file events.
> +        */
> +       if (file->f_mode & (FMODE_NONOTIFY | FMODE_PATH))
>                 return 0;
>
>         path = &file->f_path;
> --
> 2.35.3
>
Christian Brauner June 18, 2024, 2:28 p.m. UTC | #2
On Mon, 17 Jun 2024 18:23:00 +0200, Jan Kara wrote:
> Currently we will not generate FS_OPEN events for O_PATH file
> descriptors but we will generate FS_CLOSE events for them. This is
> asymmetry is confusing. Arguably no fsnotify events should be generated
> for O_PATH file descriptors as they cannot be used to access or modify
> file content, they are just convenient handles to file objects like
> paths. So fix the asymmetry by stopping to generate FS_CLOSE for O_PATH
> file descriptors.
> 
> [...]

I added a Cc stable to the first patch because this seems like a bugfix
per the mail discussion.

---

Applied to the vfs.fixes branch of the vfs/vfs.git tree.
Patches in the vfs.fixes branch should appear in linux-next soon.

Please report any outstanding bugs that were missed during review in a
new review to the original patch series allowing us to drop it.

It's encouraged to provide Acked-bys and Reviewed-bys even though the
patch has now been applied. If possible patch trailers will be updated.

Note that commit hashes shown below are subject to change due to rebase,
trailer updates or similar. If in doubt, please check the listed branch.

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git
branch: vfs.fixes

[1/2] fsnotify: Do not generate events for O_PATH file descriptors
      https://git.kernel.org/vfs/vfs/c/702eb71fd650
[2/2] vfs: generate FS_CREATE before FS_OPEN when ->atomic_open used.
      https://git.kernel.org/vfs/vfs/c/7d1cf5e624ef
Jan Kara June 19, 2024, 3:24 p.m. UTC | #3
On Tue 18-06-24 16:28:01, Christian Brauner wrote:
> On Mon, 17 Jun 2024 18:23:00 +0200, Jan Kara wrote:
> > Currently we will not generate FS_OPEN events for O_PATH file
> > descriptors but we will generate FS_CLOSE events for them. This is
> > asymmetry is confusing. Arguably no fsnotify events should be generated
> > for O_PATH file descriptors as they cannot be used to access or modify
> > file content, they are just convenient handles to file objects like
> > paths. So fix the asymmetry by stopping to generate FS_CLOSE for O_PATH
> > file descriptors.
> > 
> > [...]
> 
> I added a Cc stable to the first patch because this seems like a bugfix
> per the mail discussion.

Yes, makes sense. Thanks!

								Honza
diff mbox series

Patch

diff --git a/include/linux/fsnotify.h b/include/linux/fsnotify.h
index 4da80e92f804..278620e063ab 100644
--- a/include/linux/fsnotify.h
+++ b/include/linux/fsnotify.h
@@ -112,7 +112,13 @@  static inline int fsnotify_file(struct file *file, __u32 mask)
 {
 	const struct path *path;
 
-	if (file->f_mode & FMODE_NONOTIFY)
+	/*
+	 * FMODE_NONOTIFY are fds generated by fanotify itself which should not
+	 * generate new events. We also don't want to generate events for
+	 * FMODE_PATH fds (involves open & close events) as they are just
+	 * handle creation / destruction events and not "real" file events.
+	 */
+	if (file->f_mode & (FMODE_NONOTIFY | FMODE_PATH))
 		return 0;
 
 	path = &file->f_path;