On Tue 30-10-18 20:29:53, Amir Goldstein wrote: > When an event is reported on a sub-directory and the parent inode has > a mark mask with FS_EVENT_ON_CHILD|FS_ISDIR, the event will be sent to > fsnotify() even if the event type is not in the parent mark mask > (e.g. FS_OPEN). > > Further more, if that event happened on a mount or a filesystem with > a mount/sb mark that does have that event type in their mask, the "on > child" event will be reported on the mount/sb mark. That is not > desired, because user will get a duplicate event for the same action. > > Note that the event reported on the victim inode is never merged with > the event reported on the parent inode, because of the check in > should_merge(): old_fsn->inode == new_fsn->inode. > > Fix this by looking for a match of an actual event type (i.e. not just > FS_ISDIR) in parent's inode mark mask and by not reporting an "on child" > event to group if event type is only found on mount/sb marks. > > [backport hint: The bug seems to have always been in fanotify, but this > patch will only apply cleanly to v4.19.y] > > Cc: <stable@xxxxxxxxxxxxxxx> # v4.19 > Signed-off-by: Amir Goldstein <amir73il@xxxxxxxxx> Thanks for the patch. I've added it to my tree. Honza > --- > > Jan, > > The bugs with merging directory children mark and mount mark are > unfolding slowly. > > I have extended fanotify09, which checks another bug on this type to also > cover this case [1]. > Tested that bug existed as far back as v4.10, but I guess it was always > there. > > Thanks, > Amir. > > [1] https://github.com/amir73il/ltp/commits/fanotify_ignore > > fs/notify/fanotify/fanotify.c | 10 +++++----- > fs/notify/fsnotify.c | 7 +++++-- > 2 files changed, 10 insertions(+), 7 deletions(-) > > diff --git a/fs/notify/fanotify/fanotify.c b/fs/notify/fanotify/fanotify.c > index 5769cf3ff035..e08a6647267b 100644 > --- a/fs/notify/fanotify/fanotify.c > +++ b/fs/notify/fanotify/fanotify.c > @@ -115,12 +115,12 @@ static bool fanotify_should_send_event(struct fsnotify_iter_info *iter_info, > continue; > mark = iter_info->marks[type]; > /* > - * if the event is for a child and this inode doesn't care about > - * events on the child, don't send it! > + * If the event is for a child and this mark doesn't care about > + * events on a child, don't send it! > */ > - if (type == FSNOTIFY_OBJ_TYPE_INODE && > - (event_mask & FS_EVENT_ON_CHILD) && > - !(mark->mask & FS_EVENT_ON_CHILD)) > + if (event_mask & FS_EVENT_ON_CHILD && > + (type != FSNOTIFY_OBJ_TYPE_INODE || > + !(mark->mask & FS_EVENT_ON_CHILD))) > continue; > > marks_mask |= mark->mask; > diff --git a/fs/notify/fsnotify.c b/fs/notify/fsnotify.c > index 2172ba516c61..d2c34900ae05 100644 > --- a/fs/notify/fsnotify.c > +++ b/fs/notify/fsnotify.c > @@ -167,9 +167,9 @@ int __fsnotify_parent(const struct path *path, struct dentry *dentry, __u32 mask > parent = dget_parent(dentry); > p_inode = parent->d_inode; > > - if (unlikely(!fsnotify_inode_watches_children(p_inode))) > + if (unlikely(!fsnotify_inode_watches_children(p_inode))) { > __fsnotify_update_child_dentry_flags(p_inode); > - else if (p_inode->i_fsnotify_mask & mask) { > + } else if (p_inode->i_fsnotify_mask & mask & ALL_FSNOTIFY_EVENTS) { > struct name_snapshot name; > > /* we are notifying a parent so come up with the new mask which > @@ -339,6 +339,9 @@ int fsnotify(struct inode *to_tell, __u32 mask, const void *data, int data_is, > sb = mnt->mnt.mnt_sb; > mnt_or_sb_mask = mnt->mnt_fsnotify_mask | sb->s_fsnotify_mask; > } > + /* An event "on child" is not intended for a mount/sb mark */ > + if (mask & FS_EVENT_ON_CHILD) > + mnt_or_sb_mask = 0; > > /* > * Optimization: srcu_read_lock() has a memory barrier which can > -- > 2.17.1 > -- Jan Kara <jack@xxxxxxxx> SUSE Labs, CR
