From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm0-f66.google.com ([74.125.82.66]:34594 "EHLO mail-wm0-f66.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932417AbcL0Tcw (ORCPT ); Tue, 27 Dec 2016 14:32:52 -0500 Received: by mail-wm0-f66.google.com with SMTP id c85so20363464wmi.1 for ; Tue, 27 Dec 2016 11:32:51 -0800 (PST) From: Amir Goldstein To: Jan Kara Cc: Eric Paris , linux-fsdevel@vger.kernel.org Subject: [RFC][PATCH 1/4] fsnotify: process inode/vfsmount marks independently Date: Tue, 27 Dec 2016 21:32:25 +0200 Message-Id: <1482867148-31497-2-git-send-email-amir73il@gmail.com> In-Reply-To: <1482867148-31497-1-git-send-email-amir73il@gmail.com> References: <1482867148-31497-1-git-send-email-amir73il@gmail.com> Sender: linux-fsdevel-owner@vger.kernel.org List-ID: 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 --- 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