| From 0973bc1bacce619feca1d3598b08c4a3f9a14905 Mon Sep 17 00:00:00 2001 |
| From: Sasha Levin <sashal@kernel.org> |
| Date: Sat, 5 Sep 2026 11:44:22 +0900 |
| Subject: fsnotify: Fix stale object mask after concurrent mark updates |
| |
| From: Youngjae Kwon <yjkwon0026@snu.ac.kr> |
| |
| [ Upstream commit e422777fdd4746de1109575c51e65038d4c5c1be ] |
| |
| When a mark gets a new event bit, fanotify and inotify may 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. |
| |
| For normal fanotify additions, recalculate whenever the raw mark mask |
| changes. The normal mask is not cleared asynchronously, so an unchanged |
| addition cannot introduce missing interest. Always recalculate ignore-mask |
| updates because FS_MODIFY handling may clear the ignore mask without taking |
| mark->lock, making snapshot comparisons unreliable. |
| |
| Always recalculate after updating an existing inotify watch. Its replace |
| path temporarily sets mark->mask to zero, so a concurrent scan can observe |
| zero even when the old and final masks are equal. Assigning the replacement |
| mask directly would avoid the transient zero, but existing-watch updates |
| are infrequent, so unconditional recalculation is simpler. |
| |
| 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: stable@vger.kernel.org # needs adjustments for <= 7.0 |
| Suggested-by: Jan Kara <jack@suse.cz> |
| Suggested-by: Amir Goldstein <amir73il@gmail.com> |
| Signed-off-by: Youngjae Kwon <yjkwon0026@snu.ac.kr> |
| Link: https://patch.msgid.link/20260802015801.2426818-1-yjkwon0026@snu.ac.kr |
| Signed-off-by: Jan Kara <jack@suse.cz> |
| (cherry picked from commit e422777fdd4746de1109575c51e65038d4c5c1be) |
| [yjkwon0026: Resolve the inotify conflict by retaining the branch-native |
| inode->i_fsnotify_marks argument to fsnotify_recalc_mask(). This tree |
| lacks 35ceae44742e ("fsnotify: Avoid data race between |
| fsnotify_recalc_mask() and fsnotify_object_watched()") and |
| 4520b96b8136 ("fsnotify: inotify: pass mark connector to |
| fsnotify_recalc_mask()"). The differing READ_ONCE() line and connector |
| call are in the conditional deleted by this patch, so neither commit is |
| a prerequisite for this fix.] |
| Signed-off-by: Youngjae Kwon <yjkwon0026@snu.ac.kr> |
| Signed-off-by: Sasha Levin <sashal@kernel.org> |
| --- |
| fs/notify/fanotify/fanotify_user.c | 12 +++++++----- |
| fs/notify/inotify/inotify_user.c | 15 +-------------- |
| 2 files changed, 8 insertions(+), 19 deletions(-) |
| |
| diff --git a/fs/notify/fanotify/fanotify_user.c b/fs/notify/fanotify/fanotify_user.c |
| index 5302313f28bed..634de85a2f034 100644 |
| --- a/fs/notify/fanotify/fanotify_user.c |
| +++ b/fs/notify/fanotify/fanotify_user.c |
| @@ -1114,16 +1114,18 @@ 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; |
| bool recalc; |
| |
| spin_lock(&fsn_mark->lock); |
| - if (!(fan_flags & FANOTIFY_MARK_IGNORE_BITS)) |
| + if (!(fan_flags & FANOTIFY_MARK_IGNORE_BITS)) { |
| + old_mask = fsn_mark->mask; |
| fsn_mark->mask |= mask; |
| - else |
| + recalc = old_mask != fsn_mark->mask; |
| + } else { |
| fsn_mark->ignore_mask |= mask; |
| - |
| - recalc = fsnotify_calc_mask(fsn_mark) & |
| - ~fsnotify_conn_mask(fsn_mark->connector); |
| + recalc = true; |
| + } |
| |
| recalc |= fanotify_mark_update_flags(fsn_mark, fan_flags); |
| spin_unlock(&fsn_mark->lock); |
| diff --git a/fs/notify/inotify/inotify_user.c b/fs/notify/inotify/inotify_user.c |
| index 0b439051951cd..40343a15aeb9e 100644 |
| --- a/fs/notify/inotify/inotify_user.c |
| +++ b/fs/notify/inotify/inotify_user.c |
| @@ -527,7 +527,6 @@ static int inotify_update_existing_watch(struct fsnotify_group *group, |
| { |
| struct fsnotify_mark *fsn_mark; |
| struct inotify_inode_mark *i_mark; |
| - __u32 old_mask, new_mask; |
| int replace = !(arg & IN_MASK_ADD); |
| int create = (arg & IN_MASK_CREATE); |
| int ret; |
| @@ -543,27 +542,15 @@ static int inotify_update_existing_watch(struct fsnotify_group *group, |
| i_mark = container_of(fsn_mark, struct inotify_inode_mark, fsn_mark); |
| |
| spin_lock(&fsn_mark->lock); |
| - old_mask = fsn_mark->mask; |
| if (replace) { |
| fsn_mark->mask = 0; |
| fsn_mark->flags &= ~INOTIFY_MARK_FLAGS; |
| } |
| fsn_mark->mask |= inotify_arg_to_mask(inode, arg); |
| fsn_mark->flags |= inotify_arg_to_flags(arg); |
| - 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 & ~inode->i_fsnotify_mask); |
| - |
| - /* update the inode with this new fsn_mark */ |
| - if (dropped || do_inode) |
| - fsnotify_recalc_mask(inode->i_fsnotify_marks); |
| - |
| - } |
| + fsnotify_recalc_mask(inode->i_fsnotify_marks); |
| |
| /* return the wd */ |
| ret = i_mark->wd; |
| -- |
| 2.53.0 |
| |