mirror of
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git
synced 2024-08-26 02:39:48 +00:00
fsnotify: fix events reported to watching parent and child
commitfecc455978
upstream. fsnotify_parent() used to send two separate events to backends when a parent inode is watching children and the child inode is also watching. In an attempt to avoid duplicate events in fanotify, we unified the two backend callbacks to a single callback and handled the reporting of the two separate events for the relevant backends (inotify and dnotify). However the handling is buggy and can result in inotify and dnotify listeners receiving events of the type they never asked for or spurious events. The problem is the unified event callback with two inode marks (parent and child) is called when any of the parent and child inodes are watched and interested in the event, but the parent inode's mark that is interested in the event on the child is not necessarily the one we are currently reporting to (it could belong to a different group). So before reporting the parent or child event flavor to backend we need to check that the mark is really interested in that event flavor. The semantics of INODE and CHILD marks were hard to follow and made the logic more complicated than it should have been. Replace it with INODE and PARENT marks semantics to hopefully make the logic more clear. Thanks to Hugh Dickins for spotting a bug in the earlier version of this patch. Fixes:497b0c5a7c
("fsnotify: send event to parent and child with single callback") CC: stable@vger.kernel.org Link: https://lore.kernel.org/r/20201202120713.702387-4-amir73il@gmail.com Reported-by: Hugh Dickins <hughd@google.com> Signed-off-by: Amir Goldstein <amir73il@gmail.com> Signed-off-by: Jan Kara <jack@suse.cz> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
This commit is contained in:
parent
5e78c6bd90
commit
41bf5eed80
3 changed files with 57 additions and 40 deletions
|
@ -268,12 +268,11 @@ static u32 fanotify_group_event_mask(struct fsnotify_group *group,
|
||||||
continue;
|
continue;
|
||||||
|
|
||||||
/*
|
/*
|
||||||
* If the event is for a child and this mark is on a parent not
|
* If the event is on a child and this mark is on a parent not
|
||||||
* watching children, don't send it!
|
* watching children, don't send it!
|
||||||
*/
|
*/
|
||||||
if (event_mask & FS_EVENT_ON_CHILD &&
|
if (type == FSNOTIFY_OBJ_TYPE_PARENT &&
|
||||||
type == FSNOTIFY_OBJ_TYPE_INODE &&
|
!(mark->mask & FS_EVENT_ON_CHILD))
|
||||||
!(mark->mask & FS_EVENT_ON_CHILD))
|
|
||||||
continue;
|
continue;
|
||||||
|
|
||||||
marks_mask |= mark->mask;
|
marks_mask |= mark->mask;
|
||||||
|
|
|
@ -152,6 +152,13 @@ static bool fsnotify_event_needs_parent(struct inode *inode, struct mount *mnt,
|
||||||
if (mask & FS_ISDIR)
|
if (mask & FS_ISDIR)
|
||||||
return false;
|
return false;
|
||||||
|
|
||||||
|
/*
|
||||||
|
* All events that are possible on child can also may be reported with
|
||||||
|
* parent/name info to inode/sb/mount. Otherwise, a watching parent
|
||||||
|
* could result in events reported with unexpected name info to sb/mount.
|
||||||
|
*/
|
||||||
|
BUILD_BUG_ON(FS_EVENTS_POSS_ON_CHILD & ~FS_EVENTS_POSS_TO_PARENT);
|
||||||
|
|
||||||
/* Did either inode/sb/mount subscribe for events with parent/name? */
|
/* Did either inode/sb/mount subscribe for events with parent/name? */
|
||||||
marks_mask |= fsnotify_parent_needed_mask(inode->i_fsnotify_mask);
|
marks_mask |= fsnotify_parent_needed_mask(inode->i_fsnotify_mask);
|
||||||
marks_mask |= fsnotify_parent_needed_mask(inode->i_sb->s_fsnotify_mask);
|
marks_mask |= fsnotify_parent_needed_mask(inode->i_sb->s_fsnotify_mask);
|
||||||
|
@ -249,6 +256,10 @@ static int fsnotify_handle_inode_event(struct fsnotify_group *group,
|
||||||
path && d_unlinked(path->dentry))
|
path && d_unlinked(path->dentry))
|
||||||
return 0;
|
return 0;
|
||||||
|
|
||||||
|
/* Check interest of this mark in case event was sent with two marks */
|
||||||
|
if (!(mask & inode_mark->mask & ALL_FSNOTIFY_EVENTS))
|
||||||
|
return 0;
|
||||||
|
|
||||||
return ops->handle_inode_event(inode_mark, mask, inode, dir, name, cookie);
|
return ops->handle_inode_event(inode_mark, mask, inode, dir, name, cookie);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -258,38 +269,46 @@ static int fsnotify_handle_event(struct fsnotify_group *group, __u32 mask,
|
||||||
u32 cookie, struct fsnotify_iter_info *iter_info)
|
u32 cookie, struct fsnotify_iter_info *iter_info)
|
||||||
{
|
{
|
||||||
struct fsnotify_mark *inode_mark = fsnotify_iter_inode_mark(iter_info);
|
struct fsnotify_mark *inode_mark = fsnotify_iter_inode_mark(iter_info);
|
||||||
struct fsnotify_mark *child_mark = fsnotify_iter_child_mark(iter_info);
|
struct fsnotify_mark *parent_mark = fsnotify_iter_parent_mark(iter_info);
|
||||||
int ret;
|
int ret;
|
||||||
|
|
||||||
if (WARN_ON_ONCE(fsnotify_iter_sb_mark(iter_info)) ||
|
if (WARN_ON_ONCE(fsnotify_iter_sb_mark(iter_info)) ||
|
||||||
WARN_ON_ONCE(fsnotify_iter_vfsmount_mark(iter_info)))
|
WARN_ON_ONCE(fsnotify_iter_vfsmount_mark(iter_info)))
|
||||||
return 0;
|
return 0;
|
||||||
|
|
||||||
/*
|
if (parent_mark) {
|
||||||
* An event can be sent on child mark iterator instead of inode mark
|
/*
|
||||||
* iterator because of other groups that have interest of this inode
|
* parent_mark indicates that the parent inode is watching
|
||||||
* and have marks on both parent and child. We can simplify this case.
|
* children and interested in this event, which is an event
|
||||||
*/
|
* possible on child. But is *this mark* watching children and
|
||||||
if (!inode_mark) {
|
* interested in this event?
|
||||||
inode_mark = child_mark;
|
*/
|
||||||
child_mark = NULL;
|
if (parent_mark->mask & FS_EVENT_ON_CHILD) {
|
||||||
|
ret = fsnotify_handle_inode_event(group, parent_mark, mask,
|
||||||
|
data, data_type, dir, name, 0);
|
||||||
|
if (ret)
|
||||||
|
return ret;
|
||||||
|
}
|
||||||
|
if (!inode_mark)
|
||||||
|
return 0;
|
||||||
|
}
|
||||||
|
|
||||||
|
if (mask & FS_EVENT_ON_CHILD) {
|
||||||
|
/*
|
||||||
|
* Some events can be sent on both parent dir and child marks
|
||||||
|
* (e.g. FS_ATTRIB). If both parent dir and child are
|
||||||
|
* watching, report the event once to parent dir with name (if
|
||||||
|
* interested) and once to child without name (if interested).
|
||||||
|
* The child watcher is expecting an event without a file name
|
||||||
|
* and without the FS_EVENT_ON_CHILD flag.
|
||||||
|
*/
|
||||||
|
mask &= ~FS_EVENT_ON_CHILD;
|
||||||
dir = NULL;
|
dir = NULL;
|
||||||
name = NULL;
|
name = NULL;
|
||||||
}
|
}
|
||||||
|
|
||||||
ret = fsnotify_handle_inode_event(group, inode_mark, mask, data, data_type,
|
return fsnotify_handle_inode_event(group, inode_mark, mask, data, data_type,
|
||||||
dir, name, cookie);
|
dir, name, cookie);
|
||||||
if (ret || !child_mark)
|
|
||||||
return ret;
|
|
||||||
|
|
||||||
/*
|
|
||||||
* Some events can be sent on both parent dir and child marks
|
|
||||||
* (e.g. FS_ATTRIB). If both parent dir and child are watching,
|
|
||||||
* report the event once to parent dir with name and once to child
|
|
||||||
* without name.
|
|
||||||
*/
|
|
||||||
return fsnotify_handle_inode_event(group, child_mark, mask, data, data_type,
|
|
||||||
NULL, NULL, 0);
|
|
||||||
}
|
}
|
||||||
|
|
||||||
static int send_to_group(__u32 mask, const void *data, int data_type,
|
static int send_to_group(__u32 mask, const void *data, int data_type,
|
||||||
|
@ -447,7 +466,7 @@ int fsnotify(__u32 mask, const void *data, int data_type, struct inode *dir,
|
||||||
struct fsnotify_iter_info iter_info = {};
|
struct fsnotify_iter_info iter_info = {};
|
||||||
struct super_block *sb;
|
struct super_block *sb;
|
||||||
struct mount *mnt = NULL;
|
struct mount *mnt = NULL;
|
||||||
struct inode *child = NULL;
|
struct inode *parent = NULL;
|
||||||
int ret = 0;
|
int ret = 0;
|
||||||
__u32 test_mask, marks_mask;
|
__u32 test_mask, marks_mask;
|
||||||
|
|
||||||
|
@ -459,11 +478,10 @@ int fsnotify(__u32 mask, const void *data, int data_type, struct inode *dir,
|
||||||
inode = dir;
|
inode = dir;
|
||||||
} else if (mask & FS_EVENT_ON_CHILD) {
|
} else if (mask & FS_EVENT_ON_CHILD) {
|
||||||
/*
|
/*
|
||||||
* Event on child - report on TYPE_INODE to dir if it is
|
* Event on child - report on TYPE_PARENT to dir if it is
|
||||||
* watching children and on TYPE_CHILD to child.
|
* watching children and on TYPE_INODE to child.
|
||||||
*/
|
*/
|
||||||
child = inode;
|
parent = dir;
|
||||||
inode = dir;
|
|
||||||
}
|
}
|
||||||
sb = inode->i_sb;
|
sb = inode->i_sb;
|
||||||
|
|
||||||
|
@ -477,7 +495,7 @@ int fsnotify(__u32 mask, const void *data, int data_type, struct inode *dir,
|
||||||
if (!sb->s_fsnotify_marks &&
|
if (!sb->s_fsnotify_marks &&
|
||||||
(!mnt || !mnt->mnt_fsnotify_marks) &&
|
(!mnt || !mnt->mnt_fsnotify_marks) &&
|
||||||
(!inode || !inode->i_fsnotify_marks) &&
|
(!inode || !inode->i_fsnotify_marks) &&
|
||||||
(!child || !child->i_fsnotify_marks))
|
(!parent || !parent->i_fsnotify_marks))
|
||||||
return 0;
|
return 0;
|
||||||
|
|
||||||
marks_mask = sb->s_fsnotify_mask;
|
marks_mask = sb->s_fsnotify_mask;
|
||||||
|
@ -485,8 +503,8 @@ int fsnotify(__u32 mask, const void *data, int data_type, struct inode *dir,
|
||||||
marks_mask |= mnt->mnt_fsnotify_mask;
|
marks_mask |= mnt->mnt_fsnotify_mask;
|
||||||
if (inode)
|
if (inode)
|
||||||
marks_mask |= inode->i_fsnotify_mask;
|
marks_mask |= inode->i_fsnotify_mask;
|
||||||
if (child)
|
if (parent)
|
||||||
marks_mask |= child->i_fsnotify_mask;
|
marks_mask |= parent->i_fsnotify_mask;
|
||||||
|
|
||||||
|
|
||||||
/*
|
/*
|
||||||
|
@ -509,9 +527,9 @@ int fsnotify(__u32 mask, const void *data, int data_type, struct inode *dir,
|
||||||
iter_info.marks[FSNOTIFY_OBJ_TYPE_INODE] =
|
iter_info.marks[FSNOTIFY_OBJ_TYPE_INODE] =
|
||||||
fsnotify_first_mark(&inode->i_fsnotify_marks);
|
fsnotify_first_mark(&inode->i_fsnotify_marks);
|
||||||
}
|
}
|
||||||
if (child) {
|
if (parent) {
|
||||||
iter_info.marks[FSNOTIFY_OBJ_TYPE_CHILD] =
|
iter_info.marks[FSNOTIFY_OBJ_TYPE_PARENT] =
|
||||||
fsnotify_first_mark(&child->i_fsnotify_marks);
|
fsnotify_first_mark(&parent->i_fsnotify_marks);
|
||||||
}
|
}
|
||||||
|
|
||||||
/*
|
/*
|
||||||
|
|
|
@ -278,7 +278,7 @@ static inline const struct path *fsnotify_data_path(const void *data,
|
||||||
|
|
||||||
enum fsnotify_obj_type {
|
enum fsnotify_obj_type {
|
||||||
FSNOTIFY_OBJ_TYPE_INODE,
|
FSNOTIFY_OBJ_TYPE_INODE,
|
||||||
FSNOTIFY_OBJ_TYPE_CHILD,
|
FSNOTIFY_OBJ_TYPE_PARENT,
|
||||||
FSNOTIFY_OBJ_TYPE_VFSMOUNT,
|
FSNOTIFY_OBJ_TYPE_VFSMOUNT,
|
||||||
FSNOTIFY_OBJ_TYPE_SB,
|
FSNOTIFY_OBJ_TYPE_SB,
|
||||||
FSNOTIFY_OBJ_TYPE_COUNT,
|
FSNOTIFY_OBJ_TYPE_COUNT,
|
||||||
|
@ -286,7 +286,7 @@ enum fsnotify_obj_type {
|
||||||
};
|
};
|
||||||
|
|
||||||
#define FSNOTIFY_OBJ_TYPE_INODE_FL (1U << FSNOTIFY_OBJ_TYPE_INODE)
|
#define FSNOTIFY_OBJ_TYPE_INODE_FL (1U << FSNOTIFY_OBJ_TYPE_INODE)
|
||||||
#define FSNOTIFY_OBJ_TYPE_CHILD_FL (1U << FSNOTIFY_OBJ_TYPE_CHILD)
|
#define FSNOTIFY_OBJ_TYPE_PARENT_FL (1U << FSNOTIFY_OBJ_TYPE_PARENT)
|
||||||
#define FSNOTIFY_OBJ_TYPE_VFSMOUNT_FL (1U << FSNOTIFY_OBJ_TYPE_VFSMOUNT)
|
#define FSNOTIFY_OBJ_TYPE_VFSMOUNT_FL (1U << FSNOTIFY_OBJ_TYPE_VFSMOUNT)
|
||||||
#define FSNOTIFY_OBJ_TYPE_SB_FL (1U << FSNOTIFY_OBJ_TYPE_SB)
|
#define FSNOTIFY_OBJ_TYPE_SB_FL (1U << FSNOTIFY_OBJ_TYPE_SB)
|
||||||
#define FSNOTIFY_OBJ_ALL_TYPES_MASK ((1U << FSNOTIFY_OBJ_TYPE_COUNT) - 1)
|
#define FSNOTIFY_OBJ_ALL_TYPES_MASK ((1U << FSNOTIFY_OBJ_TYPE_COUNT) - 1)
|
||||||
|
@ -331,7 +331,7 @@ static inline struct fsnotify_mark *fsnotify_iter_##name##_mark( \
|
||||||
}
|
}
|
||||||
|
|
||||||
FSNOTIFY_ITER_FUNCS(inode, INODE)
|
FSNOTIFY_ITER_FUNCS(inode, INODE)
|
||||||
FSNOTIFY_ITER_FUNCS(child, CHILD)
|
FSNOTIFY_ITER_FUNCS(parent, PARENT)
|
||||||
FSNOTIFY_ITER_FUNCS(vfsmount, VFSMOUNT)
|
FSNOTIFY_ITER_FUNCS(vfsmount, VFSMOUNT)
|
||||||
FSNOTIFY_ITER_FUNCS(sb, SB)
|
FSNOTIFY_ITER_FUNCS(sb, SB)
|
||||||
|
|
||||||
|
|
Loading…
Reference in a new issue