Tvrtko,
On Wednesday 08 September 2010 10:24:04 Tvrtko Ursulin wrote:
>
> Improved version of the fix which does not include the check
> when permission events are not enabled in configuration and
> stops processing if no interesting events remain.
>
> Current code ignores access replies to permission decisions so
> fix it in a way which will allow all listeners to still receive
> non permission events.
I agree with the patch (see comments below), but this explanation is close
to incomprehensible and not good as a commit message.
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxxx>
> ---
> fs/notify/fsnotify.c | 32 ++++++++++++++++++++++++--------
> include/linux/fsnotify_backend.h | 4 ++++
> 2 files changed, 28 insertions(+), 8 deletions(-)
>
> diff --git a/fs/notify/fsnotify.c b/fs/notify/fsnotify.c
> index 3680242..2350a53 100644
> --- a/fs/notify/fsnotify.c
> +++ b/fs/notify/fsnotify.c
> @@ -224,7 +224,7 @@ int fsnotify(struct inode *to_tell, __u32 mask, void *data, int data_is,
> struct fsnotify_group *inode_group, *vfsmount_group;
> struct fsnotify_event *event = NULL;
> struct vfsmount *mnt;
> - int idx, ret = 0;
> + int idx, ret = 0, res;
> /* global tests shouldn't care about events on child only the specific event */
> __u32 test_mask = (mask & ~FS_EVENT_ON_CHILD);
>
> @@ -275,20 +275,36 @@ int fsnotify(struct inode *to_tell, __u32 mask, void *data, int data_is,
>
> if (inode_group > vfsmount_group) {
> /* handle inode */
> - send_to_group(to_tell, NULL, inode_mark, NULL, mask, data,
> - data_is, cookie, file_name, &event);
> + res = send_to_group(to_tell, NULL, inode_mark, NULL, mask, data,
> + data_is, cookie, file_name, &event);
> /* we didn't use the vfsmount_mark */
> vfsmount_group = NULL;
> } else if (vfsmount_group > inode_group) {
> - send_to_group(to_tell, mnt, NULL, vfsmount_mark, mask, data,
> - data_is, cookie, file_name, &event);
> + res = send_to_group(to_tell, mnt, NULL, vfsmount_mark, mask, data,
> + data_is, cookie, file_name, &event);
> inode_group = NULL;
> } else {
> - send_to_group(to_tell, mnt, inode_mark, vfsmount_mark,
> - mask, data, data_is, cookie, file_name,
> - &event);
> + res = send_to_group(to_tell, mnt, inode_mark, vfsmount_mark,
> + mask, data, data_is, cookie, file_name,
> + &event);
> }
>
> +#ifdef CONFIG_FANOTIFY_ACCESS_PERMISSIONS
> + /* If a listener denied on a permission event we will remember the reason
> + and run the rest with a non-permission mask only. This allows other
> + listeners to receive non-permission notifications but we do not care
> + about further permission checks and want to deny this event. */
> + if (unlikely((res == -EPERM) && (mask & FS_ALL_PERM_EVENTS))) {
We should probably stop processing here whenever res != 0, for any mask. (The
fanotify and inotify handle_event callbacks can return -ENOMEM right now, but
this doesn't seem very useful and should probably be fixed: fsnotify has no
way of doing anything about -ENOMEM.
> + mask &= ~FS_ALL_PERM_EVENTS;
> + ret = res;
> + /* Stop processing if no interesting events remains. */
> + if (!(mask & (ALL_FSNOTIFY_EVENTS & ~FS_EVENT_ON_CHILD)))
> + break;
> + }
> +#else
> + (void)res;
> +#endif
> +
> if (inode_group)
> inode_node = srcu_dereference(inode_node->next,
> &fsnotify_mark_srcu);
> diff --git a/include/linux/fsnotify_backend.h b/include/linux/fsnotify_backend.h
> index e40190d..c8aea03 100644
> --- a/include/linux/fsnotify_backend.h
> +++ b/include/linux/fsnotify_backend.h
> @@ -64,6 +64,10 @@
>
> #define FS_MOVE (FS_MOVED_FROM | FS_MOVED_TO)
>
> +/* All events which require a permission response from userspace */
> +#define FS_ALL_PERM_EVENTS (FS_OPEN_PERM |\
> + FS_ACCESS_PERM)
When dealing with the result of ->handle_event as I propose, this definition
isn't needed.
Thanks,
Andreas
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
