If fanotify is triggered via a vfsmount mark (so that there is
no inode mark, group in send_to_group() is set from a structure
member where the struct pointer is NULL.
This can be tested with the fanotify utility available from
http://people.redhat.com/eparis/fanotify/:
# fanotify -m / & touch /x
Signed-off-by: Csaba Henk <[email protected]>
---
fs/notify/fsnotify.c | 16 ++++++++++++----
1 files changed, 12 insertions(+), 4 deletions(-)
diff --git a/fs/notify/fsnotify.c b/fs/notify/fsnotify.c
index 3970392..6657315 100644
--- a/fs/notify/fsnotify.c
+++ b/fs/notify/fsnotify.c
@@ -148,13 +148,21 @@ static int send_to_group(struct inode *to_tell, struct vfsmount *mnt,
const unsigned char *file_name,
struct fsnotify_event **event)
{
- struct fsnotify_group *group = inode_mark->group;
+ struct fsnotify_group *group;
__u32 inode_test_mask = (mask & ~FS_EVENT_ON_CHILD);
__u32 vfsmount_test_mask = (mask & ~FS_EVENT_ON_CHILD);
- pr_debug("%s: group=%p to_tell=%p mnt=%p mark=%p mask=%x data=%p"
- " data_is=%d cookie=%d event=%p\n", __func__, group, to_tell,
- mnt, inode_mark, mask, data, data_is, cookie, *event);
+ if (inode_mark)
+ group = inode_mark->group;
+ else if (vfsmount_mark)
+ group = vfsmount_mark->group;
+ else
+ BUG();
+
+ pr_debug("%s: group=%p to_tell=%p mnt=%p inode_mark=%p vfsmount_mark=%p"
+ " mask=%x data=%p data_is=%d cookie=%d event=%p\n",
+ __func__, group, to_tell, mnt, inode_mark, vfsmount_mark, mask,
+ data, data_is, cookie, *event);
/* clear ignored on inode modification */
if (mask & FS_MODIFY) {
--
1.7.2.2
- isolate filtering logic to reduce code duplication
- the case when only one mark is passed was not handled correctly:
in this case the mask variable for the other mark was not
filtered, giving therefore a green way, regardless the mask of
the mark which was actually there to be tested
Signed-off-by: Csaba Henk <[email protected]>
---
fs/notify/fsnotify.c | 44 +++++++++++++++++---------------------------
1 files changed, 17 insertions(+), 27 deletions(-)
diff --git a/fs/notify/fsnotify.c b/fs/notify/fsnotify.c
index 6657315..ece4bd6 100644
--- a/fs/notify/fsnotify.c
+++ b/fs/notify/fsnotify.c
@@ -140,6 +140,20 @@ void __fsnotify_parent(struct path *path, struct dentry *dentry, __u32 mask)
}
EXPORT_SYMBOL_GPL(__fsnotify_parent);
+static int check_mark_mask(struct fsnotify_mark *mark, __u32 *mask)
+{
+ if (!mark)
+ return 0;
+
+ if ((*mask & FS_MODIFY) &&
+ !(mark->flags & FSNOTIFY_MARK_FLAG_IGNORED_SURV_MODIFY))
+ mark->ignored_mask = 0;
+ else
+ *mask &= ~mark->ignored_mask;
+
+ return *mask & mark->mask;
+}
+
static int send_to_group(struct inode *to_tell, struct vfsmount *mnt,
struct fsnotify_mark *inode_mark,
struct fsnotify_mark *vfsmount_mark,
@@ -149,8 +163,7 @@ static int send_to_group(struct inode *to_tell, struct vfsmount *mnt,
struct fsnotify_event **event)
{
struct fsnotify_group *group;
- __u32 inode_test_mask = (mask & ~FS_EVENT_ON_CHILD);
- __u32 vfsmount_test_mask = (mask & ~FS_EVENT_ON_CHILD);
+ __u32 test_mask = (mask & ~FS_EVENT_ON_CHILD);
if (inode_mark)
group = inode_mark->group;
@@ -164,31 +177,8 @@ static int send_to_group(struct inode *to_tell, struct vfsmount *mnt,
__func__, group, to_tell, mnt, inode_mark, vfsmount_mark, mask,
data, data_is, cookie, *event);
- /* clear ignored on inode modification */
- if (mask & FS_MODIFY) {
- if (inode_mark &&
- !(inode_mark->flags & FSNOTIFY_MARK_FLAG_IGNORED_SURV_MODIFY))
- inode_mark->ignored_mask = 0;
- if (vfsmount_mark &&
- !(vfsmount_mark->flags & FSNOTIFY_MARK_FLAG_IGNORED_SURV_MODIFY))
- vfsmount_mark->ignored_mask = 0;
- }
-
- /* does the inode mark tell us to do something? */
- if (inode_mark) {
- inode_test_mask &= inode_mark->mask;
- inode_test_mask &= ~inode_mark->ignored_mask;
- }
-
- /* does the vfsmount_mark tell us to do something? */
- if (vfsmount_mark) {
- vfsmount_test_mask &= vfsmount_mark->mask;
- vfsmount_test_mask &= ~vfsmount_mark->ignored_mask;
- if (inode_mark)
- vfsmount_test_mask &= ~inode_mark->ignored_mask;
- }
-
- if (!inode_test_mask && !vfsmount_test_mask)
+ if (!(check_mark_mask(inode_mark, &test_mask) ||
+ check_mark_mask(vfsmount_mark, &test_mask)))
return 0;
if (group->ops->should_send_event(group, to_tell, inode_mark,
--
1.7.2.2
On Sun, 2010-08-29 at 00:55 +0530, Csaba Henk wrote:
> If fanotify is triggered via a vfsmount mark (so that there is
> no inode mark, group in send_to_group() is set from a structure
> member where the struct pointer is NULL.
>
> This can be tested with the fanotify utility available from
> http://people.redhat.com/eparis/fanotify/:
>
> # fanotify -m / & touch /x
This should be fixed in the pull request I sent to Linus last night.
Sorry you had to track it down as well. There are a number of other bug
fixes in my tree
http://git.infradead.org/users/eparis/notify.git
There might still be some code duplication which something like 2/2
could clean up but your patch does apply to my devel tree and it has a
logic flaw. In the case we have both a vfsmount and an inode mark we
need to test:
event_mask & vmark->mask & ~vmark->ignored_mask & ~imark->ignored mask.
You would only ever test one or the other, not both together like that.
Thanks! Please let me know any other problems you run into!
-Eric
On Sun, Aug 29, 2010 at 2:49 AM, Eric Paris <[email protected]> wrote:
> This should be fixed in the pull request I sent to Linus last night.
> Sorry you had to track it down as well. ?There are a number of other bug
> fixes in my tree
>
> http://git.infradead.org/users/eparis/notify.git
Oh yeah, my bad, I did check that tree before getting into this... but
only the master branch, not for-linus.
> There might still be some code duplication which something like 2/2
> could clean up but your patch does apply to my devel tree and it has a
> logic flaw. ?In the case we have both a vfsmount and an inode mark we
> need to test:
>
> event_mask & vmark->mask & ~vmark->ignored_mask & ~imark->ignored mask.
>
> You would only ever test one or the other, not both together like that.
Indeed my code implements that behavior. I took care to pass the
test_mask by reference to check_mark_mask(), where upon the first
invocation it's reduced with ~imark->ignored mask, so for the second
invocation with the vmark we make test as the above expression.
If you are OK with the clean-up, I can readjust it to apply.
Csaba