| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAOQ4uxgJiiv2KrEkxwND9zpwYLakAMPX_UruGC_6Zrd5ep7duw@mail.gmail.com>
Date: Thu, 21 Nov 2024 17:01:13 +0100
From: Amir Goldstein <amir73il@...il.com>
To: Jan Kara <jack@...e.cz>
Cc: linux-fsdevel@...r.kernel.org, Josef Bacik <josef@...icpanda.com>, brauner@...nel.org,
Linus Torvalds <torvalds@...ux-foundation.org>, Al Viro <viro@...iv.linux.org.uk>,
linux-xfs@...r.kernel.org, linux-btrfs@...r.kernel.org,
linux-ext4@...r.kernel.org, linux-mm@...ck.org
Subject: Re: [PATCH 03/19] fsnotify: check if file is actually being watched
for pre-content events on open
On Thu, Nov 21, 2024 at 12:22 PM Jan Kara <jack@...e.cz> wrote:
>
> From: Amir Goldstein <amir73il@...il.com>
>
> So far, we set FMODE_NONOTIFY_ flags at open time if we know that there
> are no permission event watchers at all on the filesystem, but lack of
> FMODE_NONOTIFY_ flags does not mean that the file is actually watched.
>
> For pre-content events, it is possible to optimize things so that we
> don't bother trying to send pre-content events if file was not watched
> (through sb, mnt, parent or inode itself) on open. Set FMODE_NONOTIFY_
> flags according to that.
>
> Signed-off-by: Amir Goldstein <amir73il@...il.com>
> Signed-off-by: Jan Kara <jack@...e.cz>
> Link: https://patch.msgid.link/2ddcc9f8d1fde48d085318a6b5a889289d8871d8.1731684329.git.josef@toxicpanda.com
> ---
> fs/notify/fsnotify.c | 27 +++++++++++++++++++++++++--
> include/linux/fsnotify_backend.h | 3 +++
> 2 files changed, 28 insertions(+), 2 deletions(-)
>
> diff --git a/fs/notify/fsnotify.c b/fs/notify/fsnotify.c
> index 569ec356e4ce..dd1dffd89fd6 100644
> --- a/fs/notify/fsnotify.c
> +++ b/fs/notify/fsnotify.c
> @@ -193,7 +193,7 @@ static bool fsnotify_event_needs_parent(struct inode *inode, __u32 mnt_mask,
> return mask & marks_mask;
> }
>
> -/* Are there any inode/mount/sb objects that are interested in this event? */
> +/* Are there any inode/mount/sb objects that watch for these events? */
> static inline bool fsnotify_object_watched(struct inode *inode, __u32 mnt_mask,
> __u32 mask)
> {
> @@ -632,7 +632,9 @@ EXPORT_SYMBOL_GPL(fsnotify);
> */
> void file_set_fsnotify_mode(struct file *file)
> {
> - struct super_block *sb = file->f_path.dentry->d_sb;
> + struct dentry *dentry = file->f_path.dentry, *parent;
> + struct super_block *sb = dentry->d_sb;
> + __u32 mnt_mask, p_mask;
>
> /* Is it a file opened by fanotify? */
> if (FMODE_FSNOTIFY_NONE(file->f_mode))
> @@ -658,6 +660,27 @@ void file_set_fsnotify_mode(struct file *file)
> file->f_mode |= FMODE_NONOTIFY | FMODE_NONOTIFY_PERM;
> return;
> }
This was lost in translation:
@@ -672,8 +672,10 @@ void file_set_fsnotify_mode(struct file *file)
/*
* If there are permission event watchers but no pre-content event
* watchers, set FMODE_NONOTIFY | FMODE_NONOTIFY_PERM to indicate that.
+ * Pre-content events are only reported for regular files and dirs.
*/
- if (likely(!fsnotify_sb_has_priority_watchers(sb,
+ if ((!d_is_dir(dentry) && !d_is_reg(dentry)) ||
+ likely(!fsnotify_sb_has_priority_watchers(sb,
FSNOTIFY_PRIO_PRE_CONTENT))) {
file->f_mode |= FMODE_NONOTIFY | FMODE_NONOTIFY_PERM;
return;
> +
> + /*
> + * OK, there are some pre-content watchers. Check if anybody can be
> + * watching for pre-content events on *this* file.
> + */
> + mnt_mask = READ_ONCE(real_mount(file->f_path.mnt)->mnt_fsnotify_mask);
> + if (likely(!(dentry->d_flags & DCACHE_FSNOTIFY_PARENT_WATCHED) &&
> + !fsnotify_object_watched(d_inode(dentry), mnt_mask,
> + FSNOTIFY_PRE_CONTENT_EVENTS))) {
> + file->f_mode |= FMODE_NONOTIFY | FMODE_NONOTIFY_PERM;
> + return;
> + }
> +
> + /* Even parent is not watching for pre-content events on this file? */
> + parent = dget_parent(dentry);
> + p_mask = fsnotify_inode_watches_children(d_inode(parent));
> + dput(parent);
> + if (!(p_mask & FSNOTIFY_PRE_CONTENT_EVENTS)) {
> + file->f_mode |= FMODE_NONOTIFY | FMODE_NONOTIFY_PERM;
> + return;
> + }
inlining broke the logic and your branch fails the new PRE_ACCESS
test cases of fanotify03 LTP test (now pushed to branch fan_hsm).
Specifically in the test case that fails, parent is not watching and
inode is watching pre-content and your code gets to the p_mask
test and marks this file as no-pre-content watchers.
This passes the test:
@@ -684,17 +686,18 @@ void file_set_fsnotify_mode(struct file *file)
* watching for pre-content events on *this* file.
*/
mnt_mask = READ_ONCE(real_mount(file->f_path.mnt)->mnt_fsnotify_mask);
- if (likely(!(dentry->d_flags & DCACHE_FSNOTIFY_PARENT_WATCHED) &&
- !fsnotify_object_watched(d_inode(dentry), mnt_mask,
- FSNOTIFY_PRE_CONTENT_EVENTS))) {
- file->f_mode |= FMODE_NONOTIFY | FMODE_NONOTIFY_PERM;
+ if (unlikely(fsnotify_object_watched(d_inode(dentry), mnt_mask,
+ FSNOTIFY_PRE_CONTENT_EVENTS))) {
return;
}
/* Even parent is not watching for pre-content events on this file? */
- parent = dget_parent(dentry);
- p_mask = fsnotify_inode_watches_children(d_inode(parent));
- dput(parent);
+ p_mask = 0;
+ if (unlikely(dentry->d_flags & DCACHE_FSNOTIFY_PARENT_WATCHED)) {
+ parent = dget_parent(dentry);
+ p_mask = fsnotify_inode_watches_children(d_inode(parent));
+ dput(parent);
+ }
if (!(p_mask & FSNOTIFY_PRE_CONTENT_EVENTS)) {
file->f_mode |= FMODE_NONOTIFY | FMODE_NONOTIFY_PERM;
return;
Thanks,
Amir.
Powered by blists - more mailing lists