| Message ID | 1508421517-22678-3-git-send-email-mszeredi@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Oct 19, 2017 at 4:58 PM, Miklos Szeredi <mszeredi@redhat.com> wrote: > After having gone through a ref-unref for the mark, dereferencing the group > (e.g. in fsnotify_compare_groups()) is wrong since the group may be > completely gone by that time. So before continuing to traverse the mark > list, check if the mark is still attached. > > This is done in the generic case, not just when we go through > fsnotify_prepare_user_wait()/fsnotify_finish_user_wait(), otherwise it > would introduce unnecessary complexity. And it shouldn't hurt to skip > unattached marks anyway ("flags" is very likely in same cacheline as > neighbouring "ignored_mask", which is pulled in anyway). > Looks good. > Signed-off-by: Miklos Szeredi <mszeredi@redhat.com> > Fixes: 9385a84d7e1f ("fsnotify: Pass fsnotify_iter_info into handle_event handler") I don't know that *this* is the commit that this patch "Fixes" but for the purpose of determining if fix patch should be applied or not I guess it will do > Cc: <stable@vger.kernel.org> # v4.12 > --- > fs/notify/fsnotify.c | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/fs/notify/fsnotify.c b/fs/notify/fsnotify.c > index 48ec61f4c4d5..0ab6a7179e4d 100644 > --- a/fs/notify/fsnotify.c > +++ b/fs/notify/fsnotify.c > @@ -328,12 +328,16 @@ int fsnotify(struct inode *to_tell, __u32 mask, const void *data, int data_is, > inode_mark = hlist_entry(srcu_dereference(inode_node, &fsnotify_mark_srcu), > struct fsnotify_mark, obj_list); > inode_group = inode_mark->group; > + if (!(inode_mark->flags & FSNOTIFY_MARK_FLAG_ATTACHED)) > + goto skip_inode; > } > > if (vfsmount_node) { > vfsmount_mark = hlist_entry(srcu_dereference(vfsmount_node, &fsnotify_mark_srcu), > struct fsnotify_mark, obj_list); > vfsmount_group = vfsmount_mark->group; > + if (!(vfsmount_mark->flags & FSNOTIFY_MARK_FLAG_ATTACHED)) > + goto skip_vfsmount; > } > > iter_info.inode_mark = inode_mark; > @@ -357,10 +361,11 @@ int fsnotify(struct inode *to_tell, __u32 mask, const void *data, int data_is, > > if (ret && (mask & ALL_FSNOTIFY_PERM_EVENTS)) > goto out; > - > +skip_inode: > if (inode_group) > inode_node = srcu_dereference(inode_node->next, > &fsnotify_mark_srcu); > +skip_vfsmount: > if (vfsmount_group) > vfsmount_node = srcu_dereference(vfsmount_node->next, > &fsnotify_mark_srcu); > -- > 2.5.5 >
diff --git a/fs/notify/fsnotify.c b/fs/notify/fsnotify.c index 48ec61f4c4d5..0ab6a7179e4d 100644 --- a/fs/notify/fsnotify.c +++ b/fs/notify/fsnotify.c @@ -328,12 +328,16 @@ int fsnotify(struct inode *to_tell, __u32 mask, const void *data, int data_is, inode_mark = hlist_entry(srcu_dereference(inode_node, &fsnotify_mark_srcu), struct fsnotify_mark, obj_list); inode_group = inode_mark->group; + if (!(inode_mark->flags & FSNOTIFY_MARK_FLAG_ATTACHED)) + goto skip_inode; } if (vfsmount_node) { vfsmount_mark = hlist_entry(srcu_dereference(vfsmount_node, &fsnotify_mark_srcu), struct fsnotify_mark, obj_list); vfsmount_group = vfsmount_mark->group; + if (!(vfsmount_mark->flags & FSNOTIFY_MARK_FLAG_ATTACHED)) + goto skip_vfsmount; } iter_info.inode_mark = inode_mark; @@ -357,10 +361,11 @@ int fsnotify(struct inode *to_tell, __u32 mask, const void *data, int data_is, if (ret && (mask & ALL_FSNOTIFY_PERM_EVENTS)) goto out; - +skip_inode: if (inode_group) inode_node = srcu_dereference(inode_node->next, &fsnotify_mark_srcu); +skip_vfsmount: if (vfsmount_group) vfsmount_node = srcu_dereference(vfsmount_node->next, &fsnotify_mark_srcu);
After having gone through a ref-unref for the mark, dereferencing the group (e.g. in fsnotify_compare_groups()) is wrong since the group may be completely gone by that time. So before continuing to traverse the mark list, check if the mark is still attached. This is done in the generic case, not just when we go through fsnotify_prepare_user_wait()/fsnotify_finish_user_wait(), otherwise it would introduce unnecessary complexity. And it shouldn't hurt to skip unattached marks anyway ("flags" is very likely in same cacheline as neighbouring "ignored_mask", which is pulled in anyway). Signed-off-by: Miklos Szeredi <mszeredi@redhat.com> Fixes: 9385a84d7e1f ("fsnotify: Pass fsnotify_iter_info into handle_event handler") Cc: <stable@vger.kernel.org> # v4.12 --- fs/notify/fsnotify.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)