[RFC][PATCH 1/4] fsnotify: process inode/vfsmount marks independently

[Amir Goldstein <amir73il@gmail.com>]

Re-arrange code in send_to_group() and in fanotify_should_send_event()
so that the code processing inode_mark does not refer directly to
vfsmount_mark and vice versa.

The new code has indetical behavior to old code with one exception -
the corner case of event with FS_EVENT_ON_CHILD bit handled by fanotify
group when both inode_mark and vfsmount_mark are present and when
inode_mark->mask does not have the FS_EVENT_ON_CHILD bit set.

With old code, fanotify_should_send_event() may return true if other
event bits match the marks mask.
With new code, fanotify_should_send_event() will return false.

Normally, this event should not reach fanotify_should_send_event() at all,
because this condition is being tested earlier in __fsnotify_parent().

But even in case the event does reach fanotify_should_send_event(), the
change of behavior actually prevents the same event from being reported
twice to a group on (i.e. with and w/o FS_EVENT_ON_CHILD bit).

Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 fs/notify/fanotify/fanotify.c | 20 ++++++++------------
 fs/notify/fsnotify.c          | 15 ++++++++-------
 2 files changed, 16 insertions(+), 19 deletions(-)

diff --git a/fs/notify/fanotify/fanotify.c b/fs/notify/fanotify/fanotify.c
index bbc175d..fc7658f8 100644
--- a/fs/notify/fanotify/fanotify.c
+++ b/fs/notify/fanotify/fanotify.c
@@ -92,7 +92,7 @@ static bool fanotify_should_send_event(struct fsnotify_mark *inode_mark,
 				       u32 event_mask,
 				       const void *data, int data_type)
 {
-	__u32 marks_mask, marks_ignored_mask;
+	__u32 marks_mask = 0, marks_ignored_mask = 0;
 	const struct path *path = data;
 
 	pr_debug("%s: inode_mark=%p vfsmnt_mark=%p mask=%x data=%p"
@@ -108,10 +108,7 @@ static bool fanotify_should_send_event(struct fsnotify_mark *inode_mark,
 	    !d_can_lookup(path->dentry))
 		return false;
 
-	if (inode_mark && vfsmnt_mark) {
-		marks_mask = (vfsmnt_mark->mask | inode_mark->mask);
-		marks_ignored_mask = (vfsmnt_mark->ignored_mask | inode_mark->ignored_mask);
-	} else if (inode_mark) {
+	if (inode_mark) {
 		/*
 		 * if the event is for a child and this inode doesn't care about
 		 * events on the child, don't send it!
@@ -119,13 +116,12 @@ static bool fanotify_should_send_event(struct fsnotify_mark *inode_mark,
 		if ((event_mask & FS_EVENT_ON_CHILD) &&
 		    !(inode_mark->mask & FS_EVENT_ON_CHILD))
 			return false;
-		marks_mask = inode_mark->mask;
-		marks_ignored_mask = inode_mark->ignored_mask;
-	} else if (vfsmnt_mark) {
-		marks_mask = vfsmnt_mark->mask;
-		marks_ignored_mask = vfsmnt_mark->ignored_mask;
-	} else {
-		BUG();
+		marks_mask |= inode_mark->mask;
+		marks_ignored_mask |= inode_mark->ignored_mask;
+	}
+	if (vfsmnt_mark) {
+		marks_mask |= vfsmnt_mark->mask;
+		marks_ignored_mask |= vfsmnt_mark->ignored_mask;
 	}
 
 	if (d_is_dir(path->dentry) &&
diff --git a/fs/notify/fsnotify.c b/fs/notify/fsnotify.c
index b41515d..138e066 100644
--- a/fs/notify/fsnotify.c
+++ b/fs/notify/fsnotify.c
@@ -132,6 +132,7 @@ static int send_to_group(struct inode *to_tell,
 	struct fsnotify_group *group = NULL;
 	__u32 inode_test_mask = 0;
 	__u32 vfsmount_test_mask = 0;
+	__u32 ignored_mask = 0;
 
 	if (unlikely(!inode_mark && !vfsmount_mark)) {
 		BUG();
@@ -153,7 +154,8 @@ static int send_to_group(struct inode *to_tell,
 		group = inode_mark->group;
 		inode_test_mask = (mask & ~FS_EVENT_ON_CHILD);
 		inode_test_mask &= inode_mark->mask;
-		inode_test_mask &= ~inode_mark->ignored_mask;
+		ignored_mask |= inode_mark->ignored_mask;
+		inode_test_mask &= ~ignored_mask;
 	}
 
 	/* does the vfsmount_mark tell us to do something? */
@@ -161,17 +163,16 @@ static int send_to_group(struct inode *to_tell,
 		vfsmount_test_mask = (mask & ~FS_EVENT_ON_CHILD);
 		group = vfsmount_mark->group;
 		vfsmount_test_mask &= vfsmount_mark->mask;
-		vfsmount_test_mask &= ~vfsmount_mark->ignored_mask;
-		if (inode_mark)
-			vfsmount_test_mask &= ~inode_mark->ignored_mask;
+		ignored_mask |= vfsmount_mark->ignored_mask;
+		vfsmount_test_mask &= ~ignored_mask;
 	}
 
 	pr_debug("%s: group=%p to_tell=%p mask=%x inode_mark=%p"
 		 " inode_test_mask=%x vfsmount_mark=%p vfsmount_test_mask=%x"
-		 " data=%p data_is=%d cookie=%d\n",
+		 " ignored_mask=%x data=%p data_is=%d cookie=%d\n",
 		 __func__, group, to_tell, mask, inode_mark,
-		 inode_test_mask, vfsmount_mark, vfsmount_test_mask, data,
-		 data_is, cookie);
+		 inode_test_mask, vfsmount_mark, vfsmount_test_mask,
+		 ignored_mask, data, data_is, cookie);
 
 	if (!inode_test_mask && !vfsmount_test_mask)
 		return 0;
-- 
2.7.4