Skip to content

Commit 5c58822

Browse files
committed
Merge: CVE-2024-47660: fsnotify: clear PARENT_WATCHED flags lazily
MR: https://gitlab.com/redhat/centos-stream/src/kernel/centos-stream-9/-/merge_requests/5405 JIRA: https://issues.redhat.com/browse/RHEL-62134 CVE: CVE-2024-47660 ``` fsnotify: clear PARENT_WATCHED flags lazily In some setups directories can have many (usually negative) dentries. Hence __fsnotify_update_child_dentry_flags() function can take a significant amount of time. Since the bulk of this function happens under inode->i_lock this causes a significant contention on the lock when we remove the watch from the directory as the __fsnotify_update_child_dentry_flags() call from fsnotify_recalc_mask() races with __fsnotify_update_child_dentry_flags() calls from __fsnotify_parent() happening on children. This can lead upto softlockup reports reported by users. Fix the problem by calling fsnotify_update_children_dentry_flags() to set PARENT_WATCHED flags only when parent starts watching children. When parent stops watching children, clear false positive PARENT_WATCHED flags lazily in __fsnotify_parent() for each accessed child. Suggested-by: Jan Kara <[email protected]> Signed-off-by: Amir Goldstein <[email protected]> Signed-off-by: Stephen Brennan <[email protected]> Signed-off-by: Jan Kara <[email protected]> (cherry picked from commit 172e422) ``` Signed-off-by: CKI Backport Bot <[email protected]> --- <small>Created 2024-10-10 16:06 UTC by backporter - [KWF FAQ](https://red.ht/kernel_workflow_doc) - [Slack #team-kernel-workflow](https://redhat-internal.slack.com/archives/C04LRUPMJQ5) - [Source](https://gitlab.com/cki-project/kernel-workflow/-/blob/main/webhook/utils/backporter.py) - [Documentation](https://gitlab.com/cki-project/kernel-workflow/-/blob/main/docs/README.backporter.md) - [Report an issue](https://gitlab.com/cki-project/kernel-workflow/-/issues/new?issue%5Btitle%5D=backporter%20webhook%20issue)</small> Approved-by: Pavel Reichl <[email protected]> Approved-by: Carlos Maiolino <[email protected]> Approved-by: CKI KWF Bot <[email protected]> Merged-by: Rado Vrbovsky <[email protected]>
2 parents 05df423 + 4a2f697 commit 5c58822

File tree

4 files changed

+56
-17
lines changed

4 files changed

+56
-17
lines changed

fs/notify/fsnotify.c

Lines changed: 21 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -105,17 +105,13 @@ void fsnotify_sb_delete(struct super_block *sb)
105105
* parent cares. Thus when an event happens on a child it can quickly tell if
106106
* if there is a need to find a parent and send the event to the parent.
107107
*/
108-
void __fsnotify_update_child_dentry_flags(struct inode *inode)
108+
void fsnotify_set_children_dentry_flags(struct inode *inode)
109109
{
110110
struct dentry *alias;
111-
int watched;
112111

113112
if (!S_ISDIR(inode->i_mode))
114113
return;
115114

116-
/* determine if the children should tell inode about their events */
117-
watched = fsnotify_inode_watches_children(inode);
118-
119115
spin_lock(&inode->i_lock);
120116
/* run all of the dentries associated with this inode. Since this is a
121117
* directory, there damn well better only be one item on this list */
@@ -131,17 +127,32 @@ void __fsnotify_update_child_dentry_flags(struct inode *inode)
131127
continue;
132128

133129
spin_lock_nested(&child->d_lock, DENTRY_D_LOCK_NESTED);
134-
if (watched)
135-
child->d_flags |= DCACHE_FSNOTIFY_PARENT_WATCHED;
136-
else
137-
child->d_flags &= ~DCACHE_FSNOTIFY_PARENT_WATCHED;
130+
child->d_flags |= DCACHE_FSNOTIFY_PARENT_WATCHED;
138131
spin_unlock(&child->d_lock);
139132
}
140133
spin_unlock(&alias->d_lock);
141134
}
142135
spin_unlock(&inode->i_lock);
143136
}
144137

138+
/*
139+
* Lazily clear false positive PARENT_WATCHED flag for child whose parent had
140+
* stopped watching children.
141+
*/
142+
static void fsnotify_clear_child_dentry_flag(struct inode *pinode,
143+
struct dentry *dentry)
144+
{
145+
spin_lock(&dentry->d_lock);
146+
/*
147+
* d_lock is a sufficient barrier to prevent observing a non-watched
148+
* parent state from before the fsnotify_set_children_dentry_flags()
149+
* or fsnotify_update_flags() call that had set PARENT_WATCHED.
150+
*/
151+
if (!fsnotify_inode_watches_children(pinode))
152+
dentry->d_flags &= ~DCACHE_FSNOTIFY_PARENT_WATCHED;
153+
spin_unlock(&dentry->d_lock);
154+
}
155+
145156
/* Are inode/sb/mount interested in parent and name info with this event? */
146157
static bool fsnotify_event_needs_parent(struct inode *inode, struct mount *mnt,
147158
__u32 mask)
@@ -210,7 +221,7 @@ int __fsnotify_parent(struct dentry *dentry, __u32 mask, const void *data,
210221
p_inode = parent->d_inode;
211222
p_mask = fsnotify_inode_watches_children(p_inode);
212223
if (unlikely(parent_watched && !p_mask))
213-
__fsnotify_update_child_dentry_flags(p_inode);
224+
fsnotify_clear_child_dentry_flag(p_inode, dentry);
214225

215226
/*
216227
* Include parent/name in notification either if some notification

fs/notify/fsnotify.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ static inline void fsnotify_clear_marks_by_sb(struct super_block *sb)
7474
* update the dentry->d_flags of all of inode's children to indicate if inode cares
7575
* about events that happen to its children.
7676
*/
77-
extern void __fsnotify_update_child_dentry_flags(struct inode *inode);
77+
extern void fsnotify_set_children_dentry_flags(struct inode *inode);
7878

7979
/* allocate and destroy and event holder to attach events to notification/access queues */
8080
extern struct fsnotify_event_holder *fsnotify_alloc_event_holder(void);

fs/notify/mark.c

Lines changed: 29 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,24 @@ static void __fsnotify_recalc_mask(struct fsnotify_mark_connector *conn)
132132
*fsnotify_conn_mask_p(conn) = new_mask;
133133
}
134134

135+
static bool fsnotify_conn_watches_children(
136+
struct fsnotify_mark_connector *conn)
137+
{
138+
if (conn->type != FSNOTIFY_OBJ_TYPE_INODE)
139+
return false;
140+
141+
return fsnotify_inode_watches_children(fsnotify_conn_inode(conn));
142+
}
143+
144+
static void fsnotify_conn_set_children_dentry_flags(
145+
struct fsnotify_mark_connector *conn)
146+
{
147+
if (conn->type != FSNOTIFY_OBJ_TYPE_INODE)
148+
return;
149+
150+
fsnotify_set_children_dentry_flags(fsnotify_conn_inode(conn));
151+
}
152+
135153
/*
136154
* Calculate mask of events for a list of marks. The caller must make sure
137155
* connector and connector->obj cannot disappear under us. Callers achieve
@@ -140,15 +158,23 @@ static void __fsnotify_recalc_mask(struct fsnotify_mark_connector *conn)
140158
*/
141159
void fsnotify_recalc_mask(struct fsnotify_mark_connector *conn)
142160
{
161+
bool update_children;
162+
143163
if (!conn)
144164
return;
145165

146166
spin_lock(&conn->lock);
167+
update_children = !fsnotify_conn_watches_children(conn);
147168
__fsnotify_recalc_mask(conn);
169+
update_children &= fsnotify_conn_watches_children(conn);
148170
spin_unlock(&conn->lock);
149-
if (conn->type == FSNOTIFY_OBJ_TYPE_INODE)
150-
__fsnotify_update_child_dentry_flags(
151-
fsnotify_conn_inode(conn));
171+
/*
172+
* Set children's PARENT_WATCHED flags only if parent started watching.
173+
* When parent stops watching, we clear false positive PARENT_WATCHED
174+
* flags lazily in __fsnotify_parent().
175+
*/
176+
if (update_children)
177+
fsnotify_conn_set_children_dentry_flags(conn);
152178
}
153179

154180
/* Free all connectors queued for freeing once SRCU period ends */

include/linux/fsnotify_backend.h

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -451,12 +451,14 @@ static inline __u32 fsnotify_parent_needed_mask(__u32 mask)
451451

452452
static inline int fsnotify_inode_watches_children(struct inode *inode)
453453
{
454+
__u32 parent_mask = READ_ONCE(inode->i_fsnotify_mask);
455+
454456
/* FS_EVENT_ON_CHILD is set if the inode may care */
455-
if (!(inode->i_fsnotify_mask & FS_EVENT_ON_CHILD))
457+
if (!(parent_mask & FS_EVENT_ON_CHILD))
456458
return 0;
457459
/* this inode might care about child events, does it care about the
458460
* specific set of events that can happen on a child? */
459-
return inode->i_fsnotify_mask & FS_EVENTS_POSS_ON_CHILD;
461+
return parent_mask & FS_EVENTS_POSS_ON_CHILD;
460462
}
461463

462464
/*
@@ -470,7 +472,7 @@ static inline void fsnotify_update_flags(struct dentry *dentry)
470472
/*
471473
* Serialisation of setting PARENT_WATCHED on the dentries is provided
472474
* by d_lock. If inotify_inode_watched changes after we have taken
473-
* d_lock, the following __fsnotify_update_child_dentry_flags call will
475+
* d_lock, the following fsnotify_set_children_dentry_flags call will
474476
* find our entry, so it will spin until we complete here, and update
475477
* us with the new state.
476478
*/

0 commit comments

Comments
 (0)