* [PATCH v2 1/3] fanotify: only destroy mark when both mask and ignored_mask are cleared
@ 2014-12-04 4:43 Lino Sanfilippo
2014-12-04 4:43 ` [PATCH v2 2/3] fanotify: dont recalculate a marks mask if only the ignored mask changed Lino Sanfilippo
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Lino Sanfilippo @ 2014-12-04 4:43 UTC (permalink / raw)
To: eparis, jack; +Cc: linux-kernel, linux-fsdevel, Lino Sanfilippo
In fanotify_mark_remove_from_mask() a mark is destroyed if only one of both
bitmasks (mask or ignored_mask) of a mark is cleared. However the other mask
may still be set and contain information that should not be lost. So only
destroy a mark if both masks are cleared.
Signed-off-by: Lino Sanfilippo <LinoSanfilippo@gmx.de>
---
fs/notify/fanotify/fanotify_user.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/fs/notify/fanotify/fanotify_user.c b/fs/notify/fanotify/fanotify_user.c
index c991616..ba31055 100644
--- a/fs/notify/fanotify/fanotify_user.c
+++ b/fs/notify/fanotify/fanotify_user.c
@@ -497,10 +497,9 @@ static __u32 fanotify_mark_remove_from_mask(struct fsnotify_mark *fsn_mark,
oldmask = fsn_mark->ignored_mask;
fsnotify_set_mark_ignored_mask_locked(fsn_mark, (oldmask & ~mask));
}
+ *destroy = !(fsn_mark->mask | fsn_mark->ignored_mask);
spin_unlock(&fsn_mark->lock);
- *destroy = !(oldmask & ~mask);
-
return mask & oldmask;
}
--
1.9.1
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH v2 2/3] fanotify: dont recalculate a marks mask if only the ignored mask changed 2014-12-04 4:43 [PATCH v2 1/3] fanotify: only destroy mark when both mask and ignored_mask are cleared Lino Sanfilippo @ 2014-12-04 4:43 ` Lino Sanfilippo 2014-12-04 4:43 ` [PATCH v2 3/3] fanotify: dont set FAN_ONDIR implicitly on a marks ignored mask Lino Sanfilippo 2014-12-04 12:22 ` [PATCH v2 1/3] fanotify: only destroy mark when both mask and ignored_mask are cleared Jan Kara 2 siblings, 0 replies; 5+ messages in thread From: Lino Sanfilippo @ 2014-12-04 4:43 UTC (permalink / raw) To: eparis, jack; +Cc: linux-kernel, linux-fsdevel, Lino Sanfilippo If removing bits from a marks ignored mask, the concerning inodes/vfsmounts mask is not affected. So dont recalculate it. Signed-off-by: Lino Sanfilippo <LinoSanfilippo@gmx.de> Reviewed-by: Jan Kara <jack@suse.cz> --- fs/notify/fanotify/fanotify_user.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/fs/notify/fanotify/fanotify_user.c b/fs/notify/fanotify/fanotify_user.c index ba31055..ddc33fb 100644 --- a/fs/notify/fanotify/fanotify_user.c +++ b/fs/notify/fanotify/fanotify_user.c @@ -487,15 +487,15 @@ static __u32 fanotify_mark_remove_from_mask(struct fsnotify_mark *fsn_mark, unsigned int flags, int *destroy) { - __u32 oldmask; + __u32 oldmask = 0; spin_lock(&fsn_mark->lock); if (!(flags & FAN_MARK_IGNORED_MASK)) { oldmask = fsn_mark->mask; fsnotify_set_mark_mask_locked(fsn_mark, (oldmask & ~mask)); } else { - oldmask = fsn_mark->ignored_mask; - fsnotify_set_mark_ignored_mask_locked(fsn_mark, (oldmask & ~mask)); + __u32 tmask = fsn_mark->ignored_mask & ~mask; + fsnotify_set_mark_ignored_mask_locked(fsn_mark, tmask); } *destroy = !(fsn_mark->mask | fsn_mark->ignored_mask); spin_unlock(&fsn_mark->lock); -- 1.9.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH v2 3/3] fanotify: dont set FAN_ONDIR implicitly on a marks ignored mask 2014-12-04 4:43 [PATCH v2 1/3] fanotify: only destroy mark when both mask and ignored_mask are cleared Lino Sanfilippo 2014-12-04 4:43 ` [PATCH v2 2/3] fanotify: dont recalculate a marks mask if only the ignored mask changed Lino Sanfilippo @ 2014-12-04 4:43 ` Lino Sanfilippo 2014-12-04 12:30 ` Jan Kara 2014-12-04 12:22 ` [PATCH v2 1/3] fanotify: only destroy mark when both mask and ignored_mask are cleared Jan Kara 2 siblings, 1 reply; 5+ messages in thread From: Lino Sanfilippo @ 2014-12-04 4:43 UTC (permalink / raw) To: eparis, jack; +Cc: linux-kernel, linux-fsdevel, Lino Sanfilippo Currently FAN_ONDIR is always set on a marks ignored mask when the event mask is extended without FAN_MARK_ONDIR being set. This may result in events for directories being ignored unexpectedly for call sequences like fanotify_mark(fd, FAN_MARK_ADD, FAN_OPEN | FAN_ONDIR , AT_FDCWD, "dir"); fanotify_mark(fd, FAN_MARK_ADD, FAN_CLOSE, AT_FDCWD, "dir"); Also FAN_MARK_ONDIR is only honored when adding events to a marks mask, but not for event removal. Fix both issues. Signed-off-by: Lino Sanfilippo <LinoSanfilippo@gmx.de> --- fs/notify/fanotify/fanotify.c | 2 +- fs/notify/fanotify/fanotify_user.c | 24 ++++++++++++++++-------- 2 files changed, 17 insertions(+), 9 deletions(-) diff --git a/fs/notify/fanotify/fanotify.c b/fs/notify/fanotify/fanotify.c index 30d3add..51ceb81 100644 --- a/fs/notify/fanotify/fanotify.c +++ b/fs/notify/fanotify/fanotify.c @@ -140,7 +140,7 @@ static bool fanotify_should_send_event(struct fsnotify_mark *inode_mark, } if (S_ISDIR(path->dentry->d_inode->i_mode) && - (marks_ignored_mask & FS_ISDIR)) + !(marks_mask & FS_ISDIR & ~marks_ignored_mask)) return false; if (event_mask & marks_mask & ~marks_ignored_mask) diff --git a/fs/notify/fanotify/fanotify_user.c b/fs/notify/fanotify/fanotify_user.c index ddc33fb..683d140 100644 --- a/fs/notify/fanotify/fanotify_user.c +++ b/fs/notify/fanotify/fanotify_user.c @@ -491,10 +491,17 @@ static __u32 fanotify_mark_remove_from_mask(struct fsnotify_mark *fsn_mark, spin_lock(&fsn_mark->lock); if (!(flags & FAN_MARK_IGNORED_MASK)) { + __u32 tmask = fsn_mark->mask & ~mask; + if (flags & FAN_MARK_ONDIR) + tmask &= ~FAN_ONDIR; + oldmask = fsn_mark->mask; - fsnotify_set_mark_mask_locked(fsn_mark, (oldmask & ~mask)); + fsnotify_set_mark_mask_locked(fsn_mark, tmask); } else { __u32 tmask = fsn_mark->ignored_mask & ~mask; + if (flags & FAN_MARK_ONDIR) + tmask &= ~FAN_ONDIR; + fsnotify_set_mark_ignored_mask_locked(fsn_mark, tmask); } *destroy = !(fsn_mark->mask | fsn_mark->ignored_mask); @@ -568,20 +575,21 @@ static __u32 fanotify_mark_add_to_mask(struct fsnotify_mark *fsn_mark, spin_lock(&fsn_mark->lock); if (!(flags & FAN_MARK_IGNORED_MASK)) { + __u32 tmask = fsn_mark->mask | mask; + if (flags & FAN_MARK_ONDIR) + tmask |= FAN_ONDIR; + oldmask = fsn_mark->mask; - fsnotify_set_mark_mask_locked(fsn_mark, (oldmask | mask)); + fsnotify_set_mark_mask_locked(fsn_mark, tmask); } else { __u32 tmask = fsn_mark->ignored_mask | mask; + if (flags & FAN_MARK_ONDIR) + tmask |= FAN_ONDIR; + fsnotify_set_mark_ignored_mask_locked(fsn_mark, tmask); if (flags & FAN_MARK_IGNORED_SURV_MODIFY) fsn_mark->flags |= FSNOTIFY_MARK_FLAG_IGNORED_SURV_MODIFY; } - - if (!(flags & FAN_MARK_ONDIR)) { - __u32 tmask = fsn_mark->ignored_mask | FAN_ONDIR; - fsnotify_set_mark_ignored_mask_locked(fsn_mark, tmask); - } - spin_unlock(&fsn_mark->lock); return mask & ~oldmask; -- 1.9.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v2 3/3] fanotify: dont set FAN_ONDIR implicitly on a marks ignored mask 2014-12-04 4:43 ` [PATCH v2 3/3] fanotify: dont set FAN_ONDIR implicitly on a marks ignored mask Lino Sanfilippo @ 2014-12-04 12:30 ` Jan Kara 0 siblings, 0 replies; 5+ messages in thread From: Jan Kara @ 2014-12-04 12:30 UTC (permalink / raw) To: Lino Sanfilippo; +Cc: eparis, jack, linux-kernel, linux-fsdevel On Thu 04-12-14 05:43:04, Lino Sanfilippo wrote: > Currently FAN_ONDIR is always set on a marks ignored mask when the event mask > is extended without FAN_MARK_ONDIR being set. This may result in events for > directories being ignored unexpectedly for call sequences like > > fanotify_mark(fd, FAN_MARK_ADD, FAN_OPEN | FAN_ONDIR , AT_FDCWD, "dir"); > fanotify_mark(fd, FAN_MARK_ADD, FAN_CLOSE, AT_FDCWD, "dir"); > > Also FAN_MARK_ONDIR is only honored when adding events to a marks mask, but > not for event removal. Fix both issues. Looks good. Changelog is better but please also expand a bit "Fix both issues" to something like: Fix both issues by treating FAN_ONDIR as any other event flag and require FAN_ONDIR to be set on a mark to send events for directories. Also you can add: Reviewed-by: Jan Kara <jack@suse.cz> Honza > > Signed-off-by: Lino Sanfilippo <LinoSanfilippo@gmx.de> > --- > fs/notify/fanotify/fanotify.c | 2 +- > fs/notify/fanotify/fanotify_user.c | 24 ++++++++++++++++-------- > 2 files changed, 17 insertions(+), 9 deletions(-) > > diff --git a/fs/notify/fanotify/fanotify.c b/fs/notify/fanotify/fanotify.c > index 30d3add..51ceb81 100644 > --- a/fs/notify/fanotify/fanotify.c > +++ b/fs/notify/fanotify/fanotify.c > @@ -140,7 +140,7 @@ static bool fanotify_should_send_event(struct fsnotify_mark *inode_mark, > } > > if (S_ISDIR(path->dentry->d_inode->i_mode) && > - (marks_ignored_mask & FS_ISDIR)) > + !(marks_mask & FS_ISDIR & ~marks_ignored_mask)) > return false; > > if (event_mask & marks_mask & ~marks_ignored_mask) > diff --git a/fs/notify/fanotify/fanotify_user.c b/fs/notify/fanotify/fanotify_user.c > index ddc33fb..683d140 100644 > --- a/fs/notify/fanotify/fanotify_user.c > +++ b/fs/notify/fanotify/fanotify_user.c > @@ -491,10 +491,17 @@ static __u32 fanotify_mark_remove_from_mask(struct fsnotify_mark *fsn_mark, > > spin_lock(&fsn_mark->lock); > if (!(flags & FAN_MARK_IGNORED_MASK)) { > + __u32 tmask = fsn_mark->mask & ~mask; > + if (flags & FAN_MARK_ONDIR) > + tmask &= ~FAN_ONDIR; > + > oldmask = fsn_mark->mask; > - fsnotify_set_mark_mask_locked(fsn_mark, (oldmask & ~mask)); > + fsnotify_set_mark_mask_locked(fsn_mark, tmask); > } else { > __u32 tmask = fsn_mark->ignored_mask & ~mask; > + if (flags & FAN_MARK_ONDIR) > + tmask &= ~FAN_ONDIR; > + > fsnotify_set_mark_ignored_mask_locked(fsn_mark, tmask); > } > *destroy = !(fsn_mark->mask | fsn_mark->ignored_mask); > @@ -568,20 +575,21 @@ static __u32 fanotify_mark_add_to_mask(struct fsnotify_mark *fsn_mark, > > spin_lock(&fsn_mark->lock); > if (!(flags & FAN_MARK_IGNORED_MASK)) { > + __u32 tmask = fsn_mark->mask | mask; > + if (flags & FAN_MARK_ONDIR) > + tmask |= FAN_ONDIR; > + > oldmask = fsn_mark->mask; > - fsnotify_set_mark_mask_locked(fsn_mark, (oldmask | mask)); > + fsnotify_set_mark_mask_locked(fsn_mark, tmask); > } else { > __u32 tmask = fsn_mark->ignored_mask | mask; > + if (flags & FAN_MARK_ONDIR) > + tmask |= FAN_ONDIR; > + > fsnotify_set_mark_ignored_mask_locked(fsn_mark, tmask); > if (flags & FAN_MARK_IGNORED_SURV_MODIFY) > fsn_mark->flags |= FSNOTIFY_MARK_FLAG_IGNORED_SURV_MODIFY; > } > - > - if (!(flags & FAN_MARK_ONDIR)) { > - __u32 tmask = fsn_mark->ignored_mask | FAN_ONDIR; > - fsnotify_set_mark_ignored_mask_locked(fsn_mark, tmask); > - } > - > spin_unlock(&fsn_mark->lock); > > return mask & ~oldmask; > -- > 1.9.1 > -- Jan Kara <jack@suse.cz> SUSE Labs, CR ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2 1/3] fanotify: only destroy mark when both mask and ignored_mask are cleared 2014-12-04 4:43 [PATCH v2 1/3] fanotify: only destroy mark when both mask and ignored_mask are cleared Lino Sanfilippo 2014-12-04 4:43 ` [PATCH v2 2/3] fanotify: dont recalculate a marks mask if only the ignored mask changed Lino Sanfilippo 2014-12-04 4:43 ` [PATCH v2 3/3] fanotify: dont set FAN_ONDIR implicitly on a marks ignored mask Lino Sanfilippo @ 2014-12-04 12:22 ` Jan Kara 2 siblings, 0 replies; 5+ messages in thread From: Jan Kara @ 2014-12-04 12:22 UTC (permalink / raw) To: Lino Sanfilippo; +Cc: eparis, jack, linux-kernel, linux-fsdevel On Thu 04-12-14 05:43:02, Lino Sanfilippo wrote: > In fanotify_mark_remove_from_mask() a mark is destroyed if only one of both > bitmasks (mask or ignored_mask) of a mark is cleared. However the other mask > may still be set and contain information that should not be lost. So only > destroy a mark if both masks are cleared. > > Signed-off-by: Lino Sanfilippo <LinoSanfilippo@gmx.de> Looks good you can add: Reviewed-by: Jan Kara <jack@suse.cz> BTW: Andrew Morton merges notification patches these days so please post the patches again with reviewed-by tags and CC him as well to make it easier for him to pick them up. Honza > --- > fs/notify/fanotify/fanotify_user.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/fs/notify/fanotify/fanotify_user.c b/fs/notify/fanotify/fanotify_user.c > index c991616..ba31055 100644 > --- a/fs/notify/fanotify/fanotify_user.c > +++ b/fs/notify/fanotify/fanotify_user.c > @@ -497,10 +497,9 @@ static __u32 fanotify_mark_remove_from_mask(struct fsnotify_mark *fsn_mark, > oldmask = fsn_mark->ignored_mask; > fsnotify_set_mark_ignored_mask_locked(fsn_mark, (oldmask & ~mask)); > } > + *destroy = !(fsn_mark->mask | fsn_mark->ignored_mask); > spin_unlock(&fsn_mark->lock); > > - *destroy = !(oldmask & ~mask); > - > return mask & oldmask; > } > > -- > 1.9.1 > -- Jan Kara <jack@suse.cz> SUSE Labs, CR ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2014-12-04 12:30 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-12-04 4:43 [PATCH v2 1/3] fanotify: only destroy mark when both mask and ignored_mask are cleared Lino Sanfilippo 2014-12-04 4:43 ` [PATCH v2 2/3] fanotify: dont recalculate a marks mask if only the ignored mask changed Lino Sanfilippo 2014-12-04 4:43 ` [PATCH v2 3/3] fanotify: dont set FAN_ONDIR implicitly on a marks ignored mask Lino Sanfilippo 2014-12-04 12:30 ` Jan Kara 2014-12-04 12:22 ` [PATCH v2 1/3] fanotify: only destroy mark when both mask and ignored_mask are cleared Jan Kara
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).