On Wed 22-07-20 15:58:47, Amir Goldstein wrote: > When reporting event with parent/name info, we should not merge > parent's mark mask and ignore mask, unless the parent has the flag > FS_EVENT_ON_CHILD in the mask. > > Therefore, in fsnotify_parent(), set the FS_EVENT_ON_CHILD flag in event > mask only if parent is watching and use this flag to decide if the > parent mark masks should be merged with child/sb/mount marks. > > After this change, even groups that do not subscribe to events on > children could get an event with mark iterator type TYPE_CHILD and > without mark iterator type TYPE_INODE if fanotify has marks on the same > objects. > > dnotify and inotify event handlers can already cope with that situation. > audit does not subscribe to events that are possible on child, so won't > get to this situation. nfsd does not access the marks iterator from its > event handler at the moment, so it is not affected. > > This is a bit too fragile, so we should prepare all groups to cope with > mark type TYPE_CHILD preferably using a generic helper. > > Link: https://lore.kernel.org/linux-fsdevel/20200716223441.GA5085@xxxxxxxxxxxxxx/ > Fixes: ecf13b5f8fd6 ("fsnotify: send event with parent/name info to sb/mount/non-dir marks") > Signed-off-by: Amir Goldstein <amir73il@xxxxxxxxx> I've folded this into "fsnotify: send event with parent/name info to sb/mount/non-dir marks". Honza > --- > fs/notify/fanotify/fanotify.c | 2 +- > fs/notify/fsnotify.c | 20 +++++++++++++------- > 2 files changed, 14 insertions(+), 8 deletions(-) > > diff --git a/fs/notify/fanotify/fanotify.c b/fs/notify/fanotify/fanotify.c > index 03e3dce2a97c..3336157d895d 100644 > --- a/fs/notify/fanotify/fanotify.c > +++ b/fs/notify/fanotify/fanotify.c > @@ -538,7 +538,7 @@ static struct fanotify_event *fanotify_alloc_event(struct fsnotify_group *group, > * in addition to reporting the parent fid and maybe child name. > */ > if ((fid_mode & FAN_REPORT_FID) && > - (mask & FAN_EVENT_ON_CHILD) && !(mask & FAN_ONDIR)) > + id != dirid && !(mask & FAN_ONDIR)) > child = id; > > id = dirid; > diff --git a/fs/notify/fsnotify.c b/fs/notify/fsnotify.c > index 3b805e05c02d..494d5d70323f 100644 > --- a/fs/notify/fsnotify.c > +++ b/fs/notify/fsnotify.c > @@ -215,7 +215,8 @@ int __fsnotify_parent(struct dentry *dentry, __u32 mask, const void *data, > /* Notify both parent and child with child name info */ > take_dentry_name_snapshot(&name, dentry); > file_name = &name.name; > - mask |= FS_EVENT_ON_CHILD; > + if (parent_watched) > + mask |= FS_EVENT_ON_CHILD; > } > > notify: > @@ -391,8 +392,8 @@ int fsnotify(__u32 mask, const void *data, int data_type, struct inode *dir, > inode = dir; > } else if (mask & FS_EVENT_ON_CHILD) { > /* > - * Event on child - report on TYPE_INODE to dir > - * and on TYPE_CHILD to child. > + * Event on child - report on TYPE_INODE to dir if it is > + * watching children and on TYPE_CHILD to child. > */ > child = inode; > inode = dir; > @@ -406,14 +407,17 @@ int fsnotify(__u32 mask, const void *data, int data_type, struct inode *dir, > * SRCU because we have no references to any objects and do not > * need SRCU to keep them "alive". > */ > - if (!inode->i_fsnotify_marks && !sb->s_fsnotify_marks && > + if (!sb->s_fsnotify_marks && > (!mnt || !mnt->mnt_fsnotify_marks) && > + (!inode || !inode->i_fsnotify_marks) && > (!child || !child->i_fsnotify_marks)) > return 0; > > - marks_mask = inode->i_fsnotify_mask | sb->s_fsnotify_mask; > + marks_mask = sb->s_fsnotify_mask; > if (mnt) > marks_mask |= mnt->mnt_fsnotify_mask; > + if (inode) > + marks_mask |= inode->i_fsnotify_mask; > if (child) > marks_mask |= child->i_fsnotify_mask; > > @@ -428,14 +432,16 @@ int fsnotify(__u32 mask, const void *data, int data_type, struct inode *dir, > > iter_info.srcu_idx = srcu_read_lock(&fsnotify_mark_srcu); > > - iter_info.marks[FSNOTIFY_OBJ_TYPE_INODE] = > - fsnotify_first_mark(&inode->i_fsnotify_marks); > iter_info.marks[FSNOTIFY_OBJ_TYPE_SB] = > fsnotify_first_mark(&sb->s_fsnotify_marks); > if (mnt) { > iter_info.marks[FSNOTIFY_OBJ_TYPE_VFSMOUNT] = > fsnotify_first_mark(&mnt->mnt_fsnotify_marks); > } > + if (inode) { > + iter_info.marks[FSNOTIFY_OBJ_TYPE_INODE] = > + fsnotify_first_mark(&inode->i_fsnotify_marks); > + } > if (child) { > iter_info.marks[FSNOTIFY_OBJ_TYPE_CHILD] = > fsnotify_first_mark(&child->i_fsnotify_marks); > -- > 2.17.1 > -- Jan Kara <jack@xxxxxxxx> SUSE Labs, CR
