[PATCH] fsnotify: Fix stale object mask after concurrent mark updates
Youngjae Kwon <[email protected]>
| Newsgroups | org.kernel.vger.linux-fsdevel,org.kernel.vger.stable |
|---|---|
| Message-ID | <[email protected]> |
When a mark gets a new event bit, fanotify and inotify avoid recalculating
the object mask if the cached aggregate already contains that bit. This is
racy with a recalculation triggered by a concurrent update to another mark
on the same connector.
The concurrent scan can read the mark before the new bit is added, while
the updater reads the old aggregate before that scan publishes its result.
The updater then skips recalculation and the scan publishes a mask without
the bit, leaving the object mask stale after both updates complete.
This can be reproduced with two fanotify groups watching the same inode:
one thread removes FAN_MODIFY from one existing mark while another thread
adds FAN_MODIFY to the other mark. After both fanotify_mark() calls return,
writes can fail to produce FAN_MODIFY for the group whose mark now contains
the bit. This was reproduced on an unmodified v6.12.95 kernel. The
equivalent inotify interleaving loses IN_MODIFY events.
Always request recalculation when an existing mark's calculated mask
actually changes. For fanotify, compare fsnotify_calc_mask() before and
after normal-mask, ignore-mask, and flag updates. Keep the cached aggregate
check for unchanged updates, and keep the flag helper's inode-reference
handling.
Link: https://lore.kernel.org/all/CACwKKmCZdiZDoFuYm6LZhQ=XvHPk0fNKH=X3LmoXMqakYqJaNw@mail.gmail.com/
Fixes: 63c882a05416 ("inotify: reimplement inotify using fsnotify")
Fixes: 912ee3946c5e ("fanotify: do not call fanotify_update_object_mask in fanotify_add_mark")
Cc: [email protected] # needs adjustments for <= 7.0
Suggested-by: Jan Kara <[email protected]>
Signed-off-by: Youngjae Kwon <[email protected]>
---
I will send backports for stable kernels through 7.0.y after this lands.
fs/notify/fanotify/fanotify_user.c | 10 ++++++----
fs/notify/inotify/inotify_user.c | 13 ++-----------
2 files changed, 8 insertions(+), 15 deletions(-)
diff --git a/fs/notify/fanotify/fanotify_user.c b/fs/notify/fanotify/fanotify_user.c
index 9ee373ff5..f7080fd61 100644
--- a/fs/notify/fanotify/fanotify_user.c
+++ b/fs/notify/fanotify/fanotify_user.c
@@ -1321,18 +1321,20 @@ static bool fanotify_mark_update_flags(struct fsnotify_mark *fsn_mark,
static bool fanotify_mark_add_to_mask(struct fsnotify_mark *fsn_mark,
__u32 mask, unsigned int fan_flags)
{
+ __u32 old_mask, new_mask;
bool recalc;
spin_lock(&fsn_mark->lock);
+ old_mask = fsnotify_calc_mask(fsn_mark);
if (!(fan_flags & FANOTIFY_MARK_IGNORE_BITS))
fsn_mark->mask |= mask;
else
fsn_mark->ignore_mask |= mask;
- recalc = fsnotify_calc_mask(fsn_mark) &
- ~fsnotify_conn_mask(fsn_mark->connector);
-
- recalc |= fanotify_mark_update_flags(fsn_mark, fan_flags);
+ recalc = fanotify_mark_update_flags(fsn_mark, fan_flags);
+ new_mask = fsnotify_calc_mask(fsn_mark);
+ recalc |= old_mask != new_mask;
+ recalc |= new_mask & ~fsnotify_conn_mask(fsn_mark->connector);
spin_unlock(&fsn_mark->lock);
return recalc;
diff --git a/fs/notify/inotify/inotify_user.c b/fs/notify/inotify/inotify_user.c
index ed37491c1..c44f2daa1 100644
--- a/fs/notify/inotify/inotify_user.c
+++ b/fs/notify/inotify/inotify_user.c
@@ -565,17 +565,8 @@ static int inotify_update_existing_watch(struct fsnotify_group *group,
new_mask = fsn_mark->mask;
spin_unlock(&fsn_mark->lock);
- if (old_mask != new_mask) {
- /* more bits in old than in new? */
- int dropped = (old_mask & ~new_mask);
- /* more bits in this fsn_mark than the inode's mask? */
- int do_inode = (new_mask & ~READ_ONCE(inode->i_fsnotify_mask));
-
- /* update the inode with this new fsn_mark */
- if (dropped || do_inode)
- fsnotify_recalc_mask(fsn_mark->connector);
-
- }
+ if (old_mask != new_mask)
+ fsnotify_recalc_mask(fsn_mark->connector);
/* return the wd */
ret = i_mark->wd;
--
2.50.1