On Thu 06-11-14 08:27:08, Jan Kara wrote: > fsnotify() needs to merge inode and mount marks lists when notifying > groups about events so that ignore masks from inode marks are reflected > in mount mark notifications and groups are notified in proper order > (according to priorities). Currently the sorting of the lists done > by fsnotify_add_inode_mark() / fsnotify_add_vfsmount_mark() and > fsnotify() differed which resulted ignore masks not being used in some > cases. > > Fix the problem by always using the same comparison function when > sorting / merging the mark lists. > > Thanks to Heinrich Schuchardt for improvements of my patch. > > Link: https://bugzilla.kernel.org/show_bug.cgi?id=87721 > Reported-and-tested-by: Heinrich Schuchardt <xypron.glpk@xxxxxx> > Signed-off-by: Jan Kara <jack@xxxxxxx> BTW, I forgot to CC stable. Please add it. Thanks! Honza > --- > fs/notify/fsnotify.c | 36 +++++++++++++++++++++--------------- > fs/notify/fsnotify.h | 4 ++++ > fs/notify/inode_mark.c | 8 +++----- > fs/notify/mark.c | 36 ++++++++++++++++++++++++++++++++++++ > fs/notify/vfsmount_mark.c | 8 +++----- > 5 files changed, 67 insertions(+), 25 deletions(-) > > diff --git a/fs/notify/fsnotify.c b/fs/notify/fsnotify.c > index 9d3e9c50066a..89326acd4561 100644 > --- a/fs/notify/fsnotify.c > +++ b/fs/notify/fsnotify.c > @@ -229,8 +229,16 @@ int fsnotify(struct inode *to_tell, __u32 mask, void *data, int data_is, > &fsnotify_mark_srcu); > } > > + /* > + * We need to merge inode & vfsmount mark lists so that inode mark > + * ignore masks are properly reflected for mount mark notifications. > + * That's why this traversal is so complicated... > + */ > while (inode_node || vfsmount_node) { > - inode_group = vfsmount_group = NULL; > + inode_group = NULL; > + inode_mark = NULL; > + vfsmount_group = NULL; > + vfsmount_mark = NULL; > > if (inode_node) { > inode_mark = hlist_entry(srcu_dereference(inode_node, &fsnotify_mark_srcu), > @@ -244,21 +252,19 @@ int fsnotify(struct inode *to_tell, __u32 mask, void *data, int data_is, > vfsmount_group = vfsmount_mark->group; > } > > - if (inode_group > vfsmount_group) { > - /* handle inode */ > - ret = send_to_group(to_tell, inode_mark, NULL, mask, > - data, data_is, cookie, file_name); > - /* we didn't use the vfsmount_mark */ > - vfsmount_group = NULL; > - } else if (vfsmount_group > inode_group) { > - ret = send_to_group(to_tell, NULL, vfsmount_mark, mask, > - data, data_is, cookie, file_name); > - inode_group = NULL; > - } else { > - ret = send_to_group(to_tell, inode_mark, vfsmount_mark, > - mask, data, data_is, cookie, > - file_name); > + if (inode_group && vfsmount_group) { > + int cmp = fsnotify_compare_groups(inode_group, > + vfsmount_group); > + if (cmp > 0) { > + inode_group = NULL; > + inode_mark = NULL; > + } else if (cmp < 0) { > + vfsmount_group = NULL; > + vfsmount_mark = NULL; > + } > } > + ret = send_to_group(to_tell, inode_mark, vfsmount_mark, mask, > + data, data_is, cookie, file_name); > > if (ret && (mask & ALL_FSNOTIFY_PERM_EVENTS)) > goto out; > diff --git a/fs/notify/fsnotify.h b/fs/notify/fsnotify.h > index 9c0898c4cfe1..3b68b0ae0a97 100644 > --- a/fs/notify/fsnotify.h > +++ b/fs/notify/fsnotify.h > @@ -12,6 +12,10 @@ extern void fsnotify_flush_notify(struct fsnotify_group *group); > /* protects reads of inode and vfsmount marks list */ > extern struct srcu_struct fsnotify_mark_srcu; > > +/* compare two groups for sorting of marks lists */ > +extern int fsnotify_compare_groups(struct fsnotify_group *a, > + struct fsnotify_group *b); > + > extern void fsnotify_set_inode_mark_mask_locked(struct fsnotify_mark *fsn_mark, > __u32 mask); > /* add a mark to an inode */ > diff --git a/fs/notify/inode_mark.c b/fs/notify/inode_mark.c > index e8497144b323..dfbf5447eea4 100644 > --- a/fs/notify/inode_mark.c > +++ b/fs/notify/inode_mark.c > @@ -194,6 +194,7 @@ int fsnotify_add_inode_mark(struct fsnotify_mark *mark, > { > struct fsnotify_mark *lmark, *last = NULL; > int ret = 0; > + int cmp; > > mark->flags |= FSNOTIFY_MARK_FLAG_INODE; > > @@ -219,11 +220,8 @@ int fsnotify_add_inode_mark(struct fsnotify_mark *mark, > goto out; > } > > - if (mark->group->priority < lmark->group->priority) > - continue; > - > - if ((mark->group->priority == lmark->group->priority) && > - (mark->group < lmark->group)) > + cmp = fsnotify_compare_groups(lmark->group, mark->group); > + if (cmp < 0) > continue; > > hlist_add_before_rcu(&mark->i.i_list, &lmark->i.i_list); > diff --git a/fs/notify/mark.c b/fs/notify/mark.c > index d90deaa08e78..f7e70aac7da3 100644 > --- a/fs/notify/mark.c > +++ b/fs/notify/mark.c > @@ -210,6 +210,42 @@ void fsnotify_set_mark_ignored_mask_locked(struct fsnotify_mark *mark, __u32 mas > } > > /* > + * Sorting function for lists of fsnotify marks. > + * > + * Fanotify supports different notification classes (reflected as priority of > + * notification group). Events shall be passed to notification groups in > + * decreasing priority order. To achieve this marks in notification lists for > + * inodes and vfsmounts are sorted so that priorities of corresponding groups > + * are descending. > + * > + * Furthermore correct handling of the ignore mask requires processing inode > + * and vfsmount marks of each group together. Using the group address as > + * further sort criterion provides a unique sorting order and thus we can > + * merge inode and vfsmount lists of marks in linear time and find groups > + * present in both lists. > + * > + * A return value of 1 signifies that b has priority over a. > + * A return value of 0 signifies that the two marks have to be handled together. > + * A return value of -1 signifies that a has priority over b. > + */ > +int fsnotify_compare_groups(struct fsnotify_group *a, struct fsnotify_group *b) > +{ > + if (a == b) > + return 0; > + if (!a) > + return 1; > + if (!b) > + return -1; > + if (a->priority < b->priority) > + return 1; > + if (a->priority > b->priority) > + return -1; > + if (a < b) > + return 1; > + return -1; > +} > + > +/* > * Attach an initialized mark to a given group and fs object. > * These marks may be used for the fsnotify backend to determine which > * event types should be delivered to which group. > diff --git a/fs/notify/vfsmount_mark.c b/fs/notify/vfsmount_mark.c > index ac851e8376b1..faefa72a11eb 100644 > --- a/fs/notify/vfsmount_mark.c > +++ b/fs/notify/vfsmount_mark.c > @@ -153,6 +153,7 @@ int fsnotify_add_vfsmount_mark(struct fsnotify_mark *mark, > struct mount *m = real_mount(mnt); > struct fsnotify_mark *lmark, *last = NULL; > int ret = 0; > + int cmp; > > mark->flags |= FSNOTIFY_MARK_FLAG_VFSMOUNT; > > @@ -178,11 +179,8 @@ int fsnotify_add_vfsmount_mark(struct fsnotify_mark *mark, > goto out; > } > > - if (mark->group->priority < lmark->group->priority) > - continue; > - > - if ((mark->group->priority == lmark->group->priority) && > - (mark->group < lmark->group)) > + cmp = fsnotify_compare_groups(lmark->group, mark->group); > + if (cmp < 0) > continue; > > hlist_add_before_rcu(&mark->m.m_list, &lmark->m.m_list); > -- > 1.8.1.4 > -- Jan Kara <jack@xxxxxxx> SUSE Labs, CR -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
