fanotify: fix notification of groups with inode & mount marks

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 Signed-off-by: Jan Kara <jack@suse.cz> Reported-by: Heinrich Schuchardt <xypron.glpk@gmx.de> Tested-by: Heinrich Schuchardt <xypron.glpk@gmx.de> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

fanotify: fix notification of groups with inode & mount marks
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 Signed-off-by: Jan Kara <jack@suse.cz> Reported-by: Heinrich Schuchardt <xypron.glpk@gmx.de> Tested-by: Heinrich Schuchardt <xypron.glpk@gmx.de> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Jan Kara · Linus Torvalds
1 parent 1d5bfe1ffb
Showing 5 changed files with 67 additions and 25 deletions Side-by-side Diff
fs/notify/fsnotify.c
fs/notify/fsnotify.h
fs/notify/inode_mark.c
fs/notify/mark.c
fs/notify/vfsmount_mark.c
@@ -229,8 +229,16 @@
 					      &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 @@
 			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;
@@ -12,6 +12,10 @@
 /* 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 */
@@ -194,6 +194,7 @@
 {
 	struct fsnotify_mark *lmark, *last = NULL;
 	int ret = 0;
+	int cmp;
  
 	mark->flags |= FSNOTIFY_MARK_FLAG_INODE;
  
@@ -219,11 +220,8 @@
 			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);
@@ -210,6 +210,42 @@
 }
  
 /*
+ * 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.
@@ -153,6 +153,7 @@
 	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 @@
 			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);
...	...	@@ -229,8 +229,16 @@
229	229	&fsnotify_mark_srcu);
230	230	}
231	231
	232	+ /*
	233	+ * We need to merge inode & vfsmount mark lists so that inode mark
	234	+ * ignore masks are properly reflected for mount mark notifications.
	235	+ * That's why this traversal is so complicated...
	236	+ */
232	237	while (inode_node \|\| vfsmount_node) {
233		- inode_group = vfsmount_group = NULL;
	238	+ inode_group = NULL;
	239	+ inode_mark = NULL;
	240	+ vfsmount_group = NULL;
	241	+ vfsmount_mark = NULL;
234	242
235	243	if (inode_node) {
236	244	inode_mark = hlist_entry(srcu_dereference(inode_node, &fsnotify_mark_srcu),
237	245
...	...	@@ -244,21 +252,19 @@
244	252	vfsmount_group = vfsmount_mark->group;
245	253	}
246	254
247		- if (inode_group > vfsmount_group) {
248		- /* handle inode */
249		- ret = send_to_group(to_tell, inode_mark, NULL, mask,
250		- data, data_is, cookie, file_name);
251		- /* we didn't use the vfsmount_mark */
252		- vfsmount_group = NULL;
253		- } else if (vfsmount_group > inode_group) {
254		- ret = send_to_group(to_tell, NULL, vfsmount_mark, mask,
255		- data, data_is, cookie, file_name);
256		- inode_group = NULL;
257		- } else {
258		- ret = send_to_group(to_tell, inode_mark, vfsmount_mark,
259		- mask, data, data_is, cookie,
260		- file_name);
	255	+ if (inode_group && vfsmount_group) {
	256	+ int cmp = fsnotify_compare_groups(inode_group,
	257	+ vfsmount_group);
	258	+ if (cmp > 0) {
	259	+ inode_group = NULL;
	260	+ inode_mark = NULL;
	261	+ } else if (cmp < 0) {
	262	+ vfsmount_group = NULL;
	263	+ vfsmount_mark = NULL;
	264	+ }
261	265	}
	266	+ ret = send_to_group(to_tell, inode_mark, vfsmount_mark, mask,
	267	+ data, data_is, cookie, file_name);
262	268
263	269	if (ret && (mask & ALL_FSNOTIFY_PERM_EVENTS))
264	270	goto out;
...	...	@@ -12,6 +12,10 @@
12	12	/* protects reads of inode and vfsmount marks list */
13	13	extern struct srcu_struct fsnotify_mark_srcu;
14	14
	15	+/* compare two groups for sorting of marks lists */
	16	+extern int fsnotify_compare_groups(struct fsnotify_group *a,
	17	+ struct fsnotify_group *b);
	18	+
15	19	extern void fsnotify_set_inode_mark_mask_locked(struct fsnotify_mark *fsn_mark,
16	20	__u32 mask);
17	21	/* add a mark to an inode */
...	...	@@ -194,6 +194,7 @@
194	194	{
195	195	struct fsnotify_mark lmark, last = NULL;
196	196	int ret = 0;
	197	+ int cmp;
197	198
198	199	mark->flags \|= FSNOTIFY_MARK_FLAG_INODE;
199	200
...	...	@@ -219,11 +220,8 @@
219	220	goto out;
220	221	}
221	222
222		- if (mark->group->priority < lmark->group->priority)
223		- continue;
224		-
225		- if ((mark->group->priority == lmark->group->priority) &&
226		- (mark->group < lmark->group))
	223	+ cmp = fsnotify_compare_groups(lmark->group, mark->group);
	224	+ if (cmp < 0)
227	225	continue;
228	226
229	227	hlist_add_before_rcu(&mark->i.i_list, &lmark->i.i_list);
...	...	@@ -210,6 +210,42 @@
210	210	}
211	211
212	212	/*
	213	+ * Sorting function for lists of fsnotify marks.
	214	+ *
	215	+ * Fanotify supports different notification classes (reflected as priority of
	216	+ * notification group). Events shall be passed to notification groups in
	217	+ * decreasing priority order. To achieve this marks in notification lists for
	218	+ * inodes and vfsmounts are sorted so that priorities of corresponding groups
	219	+ * are descending.
	220	+ *
	221	+ * Furthermore correct handling of the ignore mask requires processing inode
	222	+ * and vfsmount marks of each group together. Using the group address as
	223	+ * further sort criterion provides a unique sorting order and thus we can
	224	+ * merge inode and vfsmount lists of marks in linear time and find groups
	225	+ * present in both lists.
	226	+ *
	227	+ * A return value of 1 signifies that b has priority over a.
	228	+ * A return value of 0 signifies that the two marks have to be handled together.
	229	+ * A return value of -1 signifies that a has priority over b.
	230	+ */
	231	+int fsnotify_compare_groups(struct fsnotify_group a, struct fsnotify_group b)
	232	+{
	233	+ if (a == b)
	234	+ return 0;
	235	+ if (!a)
	236	+ return 1;
	237	+ if (!b)
	238	+ return -1;
	239	+ if (a->priority < b->priority)
	240	+ return 1;
	241	+ if (a->priority > b->priority)
	242	+ return -1;
	243	+ if (a < b)
	244	+ return 1;
	245	+ return -1;
	246	+}
	247	+
	248	+/*
213	249	* Attach an initialized mark to a given group and fs object.
214	250	* These marks may be used for the fsnotify backend to determine which
215	251	* event types should be delivered to which group.
...	...	@@ -153,6 +153,7 @@
153	153	struct mount *m = real_mount(mnt);
154	154	struct fsnotify_mark lmark, last = NULL;
155	155	int ret = 0;
	156	+ int cmp;
156	157
157	158	mark->flags \|= FSNOTIFY_MARK_FLAG_VFSMOUNT;
158	159
...	...	@@ -178,11 +179,8 @@
178	179	goto out;
179	180	}
180	181
181		- if (mark->group->priority < lmark->group->priority)
182		- continue;
183		-
184		- if ((mark->group->priority == lmark->group->priority) &&
185		- (mark->group < lmark->group))
	182	+ cmp = fsnotify_compare_groups(lmark->group, mark->group);
	183	+ if (cmp < 0)
186	184	continue;
187	185
188	186	hlist_add_before_rcu(&mark->m.m_list, &lmark->m.m_list);