| 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: <20241122105807.svnmrwc3db3jqa5s@quack3>
Date: Fri, 22 Nov 2024 11:58:07 +0100
From: Jan Kara <jack@...e.cz>
To: Amir Goldstein <amir73il@...il.com>
Cc: Jan Kara <jack@...e.cz>, 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 21-11-24 17:01:13, Amir Goldstein wrote:
> 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;
Right. Specifically the (!d_is_dir(dentry) && !d_is_reg(dentry)) got lost,
likely when I moved stuff between different commits and then was resolving
patch conflicts. I'll add it back. Thanks for finding this!
> > +
> > + /*
> > + * 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;
Right. I've fixed that up. Thanks!
LTP from your fan_hsm branch is now passing so I've re-pushed the current
state to my fsnotify_hsm branch.
Honza
--
Jan Kara <jack@...e.com>
SUSE Labs, CR
Powered by blists - more mailing lists