Pull fsnotify fixes from Jan Kara: "Fixes for an inotify deadlock and a data race in fsnotify" * tag 'fsnotify_for_v6.12-rc2' of git://git.kernel.org/pub/scm/linux/kernel/git/jack/linux-fs: inotify: Fix possible deadlock in fsnotify_destroy_mark fsnotify: Avoid data race between fsnotify_recalc_mask() and fsnotify_object_watched()
Unified · Split
| @@ -792,7 +792,7 @@ nfsd_file_cache_init(void) · 791 unmodified lines | |||
| 792 | } | 792 | } |
| 793 | 793 | ||
| 794 | nfsd_file_fsnotify_group = fsnotify_alloc_group(&nfsd_file_fsnotify_ops, | 794 | nfsd_file_fsnotify_group = fsnotify_alloc_group(&nfsd_file_fsnotify_ops, |
| 795 | FSNOTIFY_GROUP_NOFS); | 795 | 0); |
| 796 | if (IS_ERR(nfsd_file_fsnotify_group)) { | 796 | if (IS_ERR(nfsd_file_fsnotify_group)) { |
| 797 | pr_err("nfsd: unable to create fsnotify group: %ld\n", | 797 | pr_err("nfsd: unable to create fsnotify group: %ld\n", |
| 798 | PTR_ERR(nfsd_file_fsnotify_group)); | 798 | PTR_ERR(nfsd_file_fsnotify_group)); |
| @@ -406,8 +406,7 @@ static int __init dnotify_init(void) · 405 unmodified lines | |||
| 406 | SLAB_PANIC|SLAB_ACCOUNT); | 406 | SLAB_PANIC|SLAB_ACCOUNT); |
| 407 | dnotify_mark_cache = KMEM_CACHE(dnotify_mark, SLAB_PANIC|SLAB_ACCOUNT); | 407 | dnotify_mark_cache = KMEM_CACHE(dnotify_mark, SLAB_PANIC|SLAB_ACCOUNT); |
| 408 | 408 | ||
| 409 | dnotify_group = fsnotify_alloc_group(&dnotify_fsnotify_ops, | 409 | dnotify_group = fsnotify_alloc_group(&dnotify_fsnotify_ops, 0); |
| 410 | FSNOTIFY_GROUP_NOFS); | ||
| 411 | if (IS_ERR(dnotify_group)) | 410 | if (IS_ERR(dnotify_group)) |
| 412 | panic("unable to allocate fsnotify group for dnotify\n"); | 411 | panic("unable to allocate fsnotify group for dnotify\n"); |
| 413 | dnotify_sysctl_init(); | 412 | dnotify_sysctl_init(); |
| @@ -1480,7 +1480,7 @@ SYSCALL_DEFINE2(fanotify_init, unsigned int, flags, unsigned int, event_f_flags) · 1479 unmodified lines | |||
| 1480 | 1480 | ||
| 1481 | /* fsnotify_alloc_group takes a ref. Dropped in fanotify_release */ | 1481 | /* fsnotify_alloc_group takes a ref. Dropped in fanotify_release */ |
| 1482 | group = fsnotify_alloc_group(&fanotify_fsnotify_ops, | 1482 | group = fsnotify_alloc_group(&fanotify_fsnotify_ops, |
| 1483 | FSNOTIFY_GROUP_USER | FSNOTIFY_GROUP_NOFS); | 1483 | FSNOTIFY_GROUP_USER); |
| 1484 | if (IS_ERR(group)) { | 1484 | if (IS_ERR(group)) { |
| 1485 | return PTR_ERR(group); | 1485 | return PTR_ERR(group); |
| 1486 | } | 1486 | } |
| @@ -183,8 +183,10 @@ static bool fsnotify_event_needs_parent(struct inode *inode, __u32 mnt_mask, · 182 unmodified lines | |||
| 183 | BUILD_BUG_ON(FS_EVENTS_POSS_ON_CHILD & ~FS_EVENTS_POSS_TO_PARENT); | 183 | BUILD_BUG_ON(FS_EVENTS_POSS_ON_CHILD & ~FS_EVENTS_POSS_TO_PARENT); |
| 184 | 184 | ||
| 185 | /* Did either inode/sb/mount subscribe for events with parent/name? */ | 185 | /* Did either inode/sb/mount subscribe for events with parent/name? */ |
| 186 | marks_mask |= fsnotify_parent_needed_mask(inode->i_fsnotify_mask); | 186 | marks_mask |= fsnotify_parent_needed_mask( |
| 187 | marks_mask |= fsnotify_parent_needed_mask(inode->i_sb->s_fsnotify_mask); | 187 | READ_ONCE(inode->i_fsnotify_mask)); |
| 188 | marks_mask |= fsnotify_parent_needed_mask( | ||
| 189 | READ_ONCE(inode->i_sb->s_fsnotify_mask)); | ||
| 188 | marks_mask |= fsnotify_parent_needed_mask(mnt_mask); | 190 | marks_mask |= fsnotify_parent_needed_mask(mnt_mask); |
| 189 | 191 | ||
| 190 | /* Did they subscribe for this event with parent/name info? */ | 192 | /* 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, · 4 unmodified lines | |||
| 195 | static inline bool fsnotify_object_watched(struct inode *inode, __u32 mnt_mask, | 197 | static inline bool fsnotify_object_watched(struct inode *inode, __u32 mnt_mask, |
| 196 | __u32 mask) | 198 | __u32 mask) |
| 197 | { | 199 | { |
| 198 | __u32 marks_mask = inode->i_fsnotify_mask | mnt_mask | | 200 | __u32 marks_mask = READ_ONCE(inode->i_fsnotify_mask) | mnt_mask | |
| 199 | inode->i_sb->s_fsnotify_mask; | 201 | READ_ONCE(inode->i_sb->s_fsnotify_mask); |
| 200 | 202 | ||
| 201 | return mask & marks_mask & ALL_FSNOTIFY_EVENTS; | 203 | return mask & marks_mask & ALL_FSNOTIFY_EVENTS; |
| 202 | } | 204 | } |
| @@ -213,7 +215,8 @@ int __fsnotify_parent(struct dentry *dentry, __u32 mask, const void *data, · 10 unmodified lines | |||
| 213 | int data_type) | 215 | int data_type) |
| 214 | { | 216 | { |
| 215 | const struct path *path = fsnotify_data_path(data, data_type); | 217 | const struct path *path = fsnotify_data_path(data, data_type); |
| 216 | __u32 mnt_mask = path ? real_mount(path->mnt)->mnt_fsnotify_mask : 0; | 218 | __u32 mnt_mask = path ? |
| 219 | READ_ONCE(real_mount(path->mnt)->mnt_fsnotify_mask) : 0; | ||
| 217 | struct inode *inode = d_inode(dentry); | 220 | struct inode *inode = d_inode(dentry); |
| 218 | struct dentry *parent; | 221 | struct dentry *parent; |
| 219 | bool parent_watched = dentry->d_flags & DCACHE_FSNOTIFY_PARENT_WATCHED; | 222 | 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, · 337 unmodified lines | |||
| 557 | (!inode2 || !inode2->i_fsnotify_marks)) | 560 | (!inode2 || !inode2->i_fsnotify_marks)) |
| 558 | return 0; | 561 | return 0; |
| 559 | 562 | ||
| 560 | marks_mask = sb->s_fsnotify_mask; | 563 | marks_mask = READ_ONCE(sb->s_fsnotify_mask); |
| 561 | if (mnt) | 564 | if (mnt) |
| 562 | marks_mask |= mnt->mnt_fsnotify_mask; | 565 | marks_mask |= READ_ONCE(mnt->mnt_fsnotify_mask); |
| 563 | if (inode) | 566 | if (inode) |
| 564 | marks_mask |= inode->i_fsnotify_mask; | 567 | marks_mask |= READ_ONCE(inode->i_fsnotify_mask); |
| 565 | if (inode2) | 568 | if (inode2) |
| 566 | marks_mask |= inode2->i_fsnotify_mask; | 569 | marks_mask |= READ_ONCE(inode2->i_fsnotify_mask); |
| 567 | 570 | ||
| 568 | 571 | ||
| 569 | /* | 572 | /* |
| @@ -115,7 +115,6 @@ static struct fsnotify_group *__fsnotify_alloc_group( · 114 unmodified lines | |||
| 115 | const struct fsnotify_ops *ops, | 115 | const struct fsnotify_ops *ops, |
| 116 | int flags, gfp_t gfp) | 116 | int flags, gfp_t gfp) |
| 117 | { | 117 | { |
| 118 | static struct lock_class_key nofs_marks_lock; | ||
| 119 | struct fsnotify_group *group; | 118 | struct fsnotify_group *group; |
| 120 | 119 | ||
| 121 | group = kzalloc(sizeof(struct fsnotify_group), gfp); | 120 | group = kzalloc(sizeof(struct fsnotify_group), gfp); |
| @@ -136,16 +135,6 @@ static struct fsnotify_group *__fsnotify_alloc_group( · 14 unmodified lines | |||
| 136 | 135 | ||
| 137 | group->ops = ops; | 136 | group->ops = ops; |
| 138 | group->flags = flags; | 137 | group->flags = flags; |
| 139 | /* | ||
| 140 | * For most backends, eviction of inode with a mark is not expected, | ||
| 141 | * because marks hold a refcount on the inode against eviction. | ||
| 142 | * | ||
| 143 | * Use a different lockdep class for groups that support evictable | ||
| 144 | * inode marks, because with evictable marks, mark_mutex is NOT | ||
| 145 | * fs-reclaim safe - the mutex is taken when evicting inodes. | ||
| 146 | */ | ||
| 147 | if (flags & FSNOTIFY_GROUP_NOFS) | ||
| 148 | lockdep_set_class(&group->mark_mutex, &nofs_marks_lock); | ||
| 149 | 138 | ||
| 150 | return group; | 139 | return group; |
| 151 | } | 140 | } |
| @@ -569,7 +569,7 @@ static int inotify_update_existing_watch(struct fsnotify_group *group, · 568 unmodified lines | |||
| 569 | /* more bits in old than in new? */ | 569 | /* more bits in old than in new? */ |
| 570 | int dropped = (old_mask & ~new_mask); | 570 | int dropped = (old_mask & ~new_mask); |
| 571 | /* more bits in this fsn_mark than the inode's mask? */ | 571 | /* more bits in this fsn_mark than the inode's mask? */ |
| 572 | int do_inode = (new_mask & ~inode->i_fsnotify_mask); | 572 | int do_inode = (new_mask & ~READ_ONCE(inode->i_fsnotify_mask)); |
| 573 | 573 | ||
| 574 | /* update the inode with this new fsn_mark */ | 574 | /* update the inode with this new fsn_mark */ |
| 575 | if (dropped || do_inode) | 575 | if (dropped || do_inode) |
| @@ -128,7 +128,7 @@ __u32 fsnotify_conn_mask(struct fsnotify_mark_connector *conn) · 127 unmodified lines | |||
| 128 | if (WARN_ON(!fsnotify_valid_obj_type(conn->type))) | 128 | if (WARN_ON(!fsnotify_valid_obj_type(conn->type))) |
| 129 | return 0; | 129 | return 0; |
| 130 | 130 | ||
| 131 | return *fsnotify_conn_mask_p(conn); | 131 | return READ_ONCE(*fsnotify_conn_mask_p(conn)); |
| 132 | } | 132 | } |
| 133 | 133 | ||
| 134 | static void fsnotify_get_sb_watched_objects(struct super_block *sb) | 134 | 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) · 110 unmodified lines | |||
| 245 | !(mark->flags & FSNOTIFY_MARK_FLAG_NO_IREF)) | 245 | !(mark->flags & FSNOTIFY_MARK_FLAG_NO_IREF)) |
| 246 | want_iref = true; | 246 | want_iref = true; |
| 247 | } | 247 | } |
| 248 | *fsnotify_conn_mask_p(conn) = new_mask; | 248 | /* |
| 249 | * We use WRITE_ONCE() to prevent silly compiler optimizations from | ||
| 250 | * confusing readers not holding conn->lock with partial updates. | ||
| 251 | */ | ||
| 252 | WRITE_ONCE(*fsnotify_conn_mask_p(conn), new_mask); | ||
| 249 | 253 | ||
| 250 | return fsnotify_update_iref(conn, want_iref); | 254 | return fsnotify_update_iref(conn, want_iref); |
| 251 | } | 255 | } |
| @@ -217,7 +217,6 @@ struct fsnotify_group { · 216 unmodified lines | |||
| 217 | 217 | ||
| 218 | #define FSNOTIFY_GROUP_USER 0x01 /* user allocated group */ | 218 | #define FSNOTIFY_GROUP_USER 0x01 /* user allocated group */ |
| 219 | #define FSNOTIFY_GROUP_DUPS 0x02 /* allow multiple marks per object */ | 219 | #define FSNOTIFY_GROUP_DUPS 0x02 /* allow multiple marks per object */ |
| 220 | #define FSNOTIFY_GROUP_NOFS 0x04 /* group lock is not direct reclaim safe */ | ||
| 221 | int flags; | 220 | int flags; |
| 222 | unsigned int owner_flags; /* stored flags of mark_mutex owner */ | 221 | unsigned int owner_flags; /* stored flags of mark_mutex owner */ |
| 223 | 222 | ||
| @@ -268,22 +267,19 @@ struct fsnotify_group { · 44 unmodified lines | |||
| 268 | static inline void fsnotify_group_lock(struct fsnotify_group *group) | 267 | static inline void fsnotify_group_lock(struct fsnotify_group *group) |
| 269 | { | 268 | { |
| 270 | mutex_lock(&group->mark_mutex); | 269 | mutex_lock(&group->mark_mutex); |
| 271 | if (group->flags & FSNOTIFY_GROUP_NOFS) | 270 | group->owner_flags = memalloc_nofs_save(); |
| 272 | group->owner_flags = memalloc_nofs_save(); | ||
| 273 | } | 271 | } |
| 274 | 272 | ||
| 275 | static inline void fsnotify_group_unlock(struct fsnotify_group *group) | 273 | static inline void fsnotify_group_unlock(struct fsnotify_group *group) |
| 276 | { | 274 | { |
| 277 | if (group->flags & FSNOTIFY_GROUP_NOFS) | 275 | memalloc_nofs_restore(group->owner_flags); |
| 278 | memalloc_nofs_restore(group->owner_flags); | ||
| 279 | mutex_unlock(&group->mark_mutex); | 276 | mutex_unlock(&group->mark_mutex); |
| 280 | } | 277 | } |
| 281 | 278 | ||
| 282 | static inline void fsnotify_group_assert_locked(struct fsnotify_group *group) | 279 | static inline void fsnotify_group_assert_locked(struct fsnotify_group *group) |
| 283 | { | 280 | { |
| 284 | WARN_ON_ONCE(!mutex_is_locked(&group->mark_mutex)); | 281 | WARN_ON_ONCE(!mutex_is_locked(&group->mark_mutex)); |
| 285 | if (group->flags & FSNOTIFY_GROUP_NOFS) | 282 | WARN_ON_ONCE(!(current->flags & PF_MEMALLOC_NOFS)); |
| 286 | WARN_ON_ONCE(!(current->flags & PF_MEMALLOC_NOFS)); | ||
| 287 | } | 283 | } |
| 288 | 284 | ||
| 289 | /* When calling fsnotify tell it if the data is a path or inode */ | 285 | /* When calling fsnotify tell it if the data is a path or inode */ |