This is a note to let you know that I've just added the patch titled fsnotify: Avoid data race between fsnotify_recalc_mask() and fsnotify_object_watched() to the 6.11-stable tree which can be found at: http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary The filename of the patch is: fsnotify-avoid-data-race-between-fsnotify_recalc_mas.patch and it can be found in the queue-6.11 subdirectory. If you, or anyone else, feels it should not be added to the stable tree, please let <stable@xxxxxxxxxxxxxxx> know about it. commit af603e3969ff4dbbed40ab86b65a2488b9db9bd4 Author: Jan Kara <jack@xxxxxxx> Date: Wed Jul 17 16:06:23 2024 +0200 fsnotify: Avoid data race between fsnotify_recalc_mask() and fsnotify_object_watched() [ Upstream commit 35ceae44742e1101f9d20adadbbbd92c05d7d659 ] When __fsnotify_recalc_mask() recomputes the mask on the watched object, the compiler can "optimize" the code to perform partial updates to the mask (including zeroing it at the beginning). Thus places checking the object mask without conn->lock such as fsnotify_object_watched() could see invalid states of the mask. Make sure the mask update is performed by one memory store using WRITE_ONCE(). Reported-by: syzbot+701037856c25b143f1ad@xxxxxxxxxxxxxxxxxxxxxxxxx Reported-by: Dmitry Vyukov <dvyukov@xxxxxxxxxx> Link: https://lore.kernel.org/all/CACT4Y+Zk0ohwwwHSD63U2-PQ=UuamXczr1mKBD6xtj2dyYKBvA@xxxxxxxxxxxxxx Signed-off-by: Jan Kara <jack@xxxxxxx> Reviewed-by: Josef Bacik <josef@xxxxxxxxxxxxxx> Link: https://patch.msgid.link/20240717140623.27768-1-jack@xxxxxxx Signed-off-by: Sasha Levin <sashal@xxxxxxxxxx> diff --git a/fs/notify/fsnotify.c b/fs/notify/fsnotify.c index 272c8a1dab3c2..82ae8254c068b 100644 --- a/fs/notify/fsnotify.c +++ b/fs/notify/fsnotify.c @@ -183,8 +183,10 @@ static bool fsnotify_event_needs_parent(struct inode *inode, __u32 mnt_mask, BUILD_BUG_ON(FS_EVENTS_POSS_ON_CHILD & ~FS_EVENTS_POSS_TO_PARENT); /* 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_sb->s_fsnotify_mask); + marks_mask |= fsnotify_parent_needed_mask( + READ_ONCE(inode->i_fsnotify_mask)); + marks_mask |= fsnotify_parent_needed_mask( + READ_ONCE(inode->i_sb->s_fsnotify_mask)); marks_mask |= fsnotify_parent_needed_mask(mnt_mask); /* Did they subscribe for this event with parent/name info? */ @@ -195,8 +197,8 @@ static bool fsnotify_event_needs_parent(struct inode *inode, __u32 mnt_mask, static inline bool fsnotify_object_watched(struct inode *inode, __u32 mnt_mask, __u32 mask) { - __u32 marks_mask = inode->i_fsnotify_mask | mnt_mask | - inode->i_sb->s_fsnotify_mask; + __u32 marks_mask = READ_ONCE(inode->i_fsnotify_mask) | mnt_mask | + READ_ONCE(inode->i_sb->s_fsnotify_mask); return mask & marks_mask & ALL_FSNOTIFY_EVENTS; } @@ -213,7 +215,8 @@ int __fsnotify_parent(struct dentry *dentry, __u32 mask, const void *data, int data_type) { const struct path *path = fsnotify_data_path(data, data_type); - __u32 mnt_mask = path ? real_mount(path->mnt)->mnt_fsnotify_mask : 0; + __u32 mnt_mask = path ? + READ_ONCE(real_mount(path->mnt)->mnt_fsnotify_mask) : 0; struct inode *inode = d_inode(dentry); struct dentry *parent; bool parent_watched = dentry->d_flags & DCACHE_FSNOTIFY_PARENT_WATCHED; @@ -557,13 +560,13 @@ int fsnotify(__u32 mask, const void *data, int data_type, struct inode *dir, (!inode2 || !inode2->i_fsnotify_marks)) return 0; - marks_mask = sb->s_fsnotify_mask; + marks_mask = READ_ONCE(sb->s_fsnotify_mask); if (mnt) - marks_mask |= mnt->mnt_fsnotify_mask; + marks_mask |= READ_ONCE(mnt->mnt_fsnotify_mask); if (inode) - marks_mask |= inode->i_fsnotify_mask; + marks_mask |= READ_ONCE(inode->i_fsnotify_mask); if (inode2) - marks_mask |= inode2->i_fsnotify_mask; + marks_mask |= READ_ONCE(inode2->i_fsnotify_mask); /* diff --git a/fs/notify/inotify/inotify_user.c b/fs/notify/inotify/inotify_user.c index 4ffc30606e0b9..e163a4b790224 100644 --- a/fs/notify/inotify/inotify_user.c +++ b/fs/notify/inotify/inotify_user.c @@ -569,7 +569,7 @@ static int inotify_update_existing_watch(struct fsnotify_group *group, /* 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); + int do_inode = (new_mask & ~READ_ONCE(inode->i_fsnotify_mask)); /* update the inode with this new fsn_mark */ if (dropped || do_inode) diff --git a/fs/notify/mark.c b/fs/notify/mark.c index 5e170e7130886..c45b222cf9c11 100644 --- a/fs/notify/mark.c +++ b/fs/notify/mark.c @@ -128,7 +128,7 @@ __u32 fsnotify_conn_mask(struct fsnotify_mark_connector *conn) if (WARN_ON(!fsnotify_valid_obj_type(conn->type))) return 0; - return *fsnotify_conn_mask_p(conn); + return READ_ONCE(*fsnotify_conn_mask_p(conn)); } static void fsnotify_get_sb_watched_objects(struct super_block *sb) @@ -245,7 +245,11 @@ static void *__fsnotify_recalc_mask(struct fsnotify_mark_connector *conn) !(mark->flags & FSNOTIFY_MARK_FLAG_NO_IREF)) want_iref = true; } - *fsnotify_conn_mask_p(conn) = new_mask; + /* + * We use WRITE_ONCE() to prevent silly compiler optimizations from + * confusing readers not holding conn->lock with partial updates. + */ + WRITE_ONCE(*fsnotify_conn_mask_p(conn), new_mask); return fsnotify_update_iref(conn, want_iref); }