fanotify: fix handling of events on child sub-directory
Amir Goldstein
reviewerPATCH 1/1 fanotify: fix handling of events on child sub-directory
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]
---
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(-)
fs/notify/fanotify/fanotify.c +5 -5
@@ -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;
fs/notify/fsnotify.c +5 -2
@@ -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
Signature · 1 line
2.17.1
Jan Kara
maintainerQuoted 18 lines · Amir Goldstein
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.
Quoted 5 lines
[backport hint: The bug seems to have always been in fanotify, but this
patch will only apply cleanly to v4.19.y]Cc <[email protected]> # v4.19Signed-off-by Amir Goldstein <[email protected]>
Thanks for the patch. I've added it to my tree.
Honza
Quoted 72 lines
---
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(-)fs/notify/fanotify/fanotify.c +5
-5@@ -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;fs/notify/fsnotify.c +5
-2@@ -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 Signature · 1 line
2.17.1
Signature · 2 lines
Jan Kara <[email protected]> SUSE Labs, CR