On Wed, Feb 13, 2019 at 4:54 PM Jan Kara <jack@xxxxxxx> wrote:
>
> Track whether permission event got already reported to userspace and
> whether userspace already answered to the permission event. Protect
> stores to this field together with updates to ->response field by
> group->notification_lock. This will allow aborting wait for reply to
> permission event from userspace.
>
> Signed-off-by: Jan Kara <jack@xxxxxxx>
Reviewed-by: Amir Goldstein <amir73il@xxxxxxxxx>
> ---
> fs/notify/fanotify/fanotify.c | 6 +++---
> fs/notify/fanotify/fanotify.h | 10 +++++++++-
> fs/notify/fanotify/fanotify_user.c | 35 ++++++++++++++++++++++++++++-------
> 3 files changed, 40 insertions(+), 11 deletions(-)
>
> diff --git a/fs/notify/fanotify/fanotify.c b/fs/notify/fanotify/fanotify.c
> index 3723f3d18d20..e725831be161 100644
> --- a/fs/notify/fanotify/fanotify.c
> +++ b/fs/notify/fanotify/fanotify.c
> @@ -65,7 +65,8 @@ static int fanotify_get_response(struct fsnotify_group *group,
>
> pr_debug("%s: group=%p event=%p\n", __func__, group, event);
>
> - wait_event(group->fanotify_data.access_waitq, event->response);
> + wait_event(group->fanotify_data.access_waitq,
> + event->state == FAN_EVENT_ANSWERED);
>
> /* userspace responded, convert to something usable */
> switch (event->response & ~FAN_AUDIT) {
> @@ -81,8 +82,6 @@ static int fanotify_get_response(struct fsnotify_group *group,
> if (event->response & FAN_AUDIT)
> audit_fanotify(event->response & ~FAN_AUDIT);
>
> - event->response = 0;
> -
> pr_debug("%s: group=%p event=%p about to return ret=%d\n", __func__,
> group, event, ret);
>
> @@ -167,6 +166,7 @@ struct fanotify_event_info *fanotify_alloc_event(struct fsnotify_group *group,
> goto out;
> event = &pevent->fae;
> pevent->response = 0;
> + pevent->state = FAN_EVENT_INIT;
> goto init;
> }
> event = kmem_cache_alloc(fanotify_event_cachep, gfp);
> diff --git a/fs/notify/fanotify/fanotify.h b/fs/notify/fanotify/fanotify.h
> index ea05b8a401e7..98d58939777c 100644
> --- a/fs/notify/fanotify/fanotify.h
> +++ b/fs/notify/fanotify/fanotify.h
> @@ -22,6 +22,13 @@ struct fanotify_event_info {
> struct pid *pid;
> };
>
> +/* Possible states of the permission event */
> +enum {
> + FAN_EVENT_INIT,
> + FAN_EVENT_REPORTED,
> + FAN_EVENT_ANSWERED
> +};
> +
> /*
> * Structure for permission fanotify events. It gets allocated and freed in
> * fanotify_handle_event() since we wait there for user response. When the
> @@ -31,7 +38,8 @@ struct fanotify_event_info {
> */
> struct fanotify_perm_event_info {
> struct fanotify_event_info fae;
> - int response; /* userspace answer to question */
> + unsigned short response; /* userspace answer to the event */
> + unsigned short state; /* state of the event */
> int fd; /* fd we passed to userspace for this event */
> };
>
> diff --git a/fs/notify/fanotify/fanotify_user.c b/fs/notify/fanotify/fanotify_user.c
> index 96dc469a4086..ef6bea0ae751 100644
> --- a/fs/notify/fanotify/fanotify_user.c
> +++ b/fs/notify/fanotify/fanotify_user.c
> @@ -50,7 +50,8 @@ struct kmem_cache *fanotify_perm_event_cachep __read_mostly;
> /*
> * Get an fsnotify notification event if one exists and is small
> * enough to fit in "count". Return an error pointer if the count
> - * is not large enough.
> + * is not large enough. When permission event is dequeued, its state is
> + * updated accordingly.
> */
> static struct fsnotify_event *get_one_event(struct fsnotify_group *group,
> size_t count)
> @@ -67,6 +68,8 @@ static struct fsnotify_event *get_one_event(struct fsnotify_group *group,
> goto out;
> }
> event = fsnotify_remove_first_event(group);
> + if (fanotify_is_perm_event(event->mask))
> + FANOTIFY_PE(event)->state = FAN_EVENT_REPORTED;
> out:
> spin_unlock(&group->notification_lock);
> return event;
> @@ -144,6 +147,21 @@ static int fill_event_metadata(struct fsnotify_group *group,
> return ret;
> }
>
> +/*
> + * Finish processing of permission event by setting it to ANSWERED state and
> + * drop group->notification_lock.
> + */
> +static void finish_permission_event(struct fsnotify_group *group,
> + struct fanotify_perm_event_info *event,
> + unsigned int response)
> + __releases(&group->notification_lock)
> +{
> + assert_spin_locked(&group->notification_lock);
> + event->response = response;
> + event->state = FAN_EVENT_ANSWERED;
> + spin_unlock(&group->notification_lock);
> +}
> +
> static int process_access_response(struct fsnotify_group *group,
> struct fanotify_response *response_struct)
> {
> @@ -179,8 +197,7 @@ static int process_access_response(struct fsnotify_group *group,
> continue;
>
> list_del_init(&event->fae.fse.list);
> - event->response = response;
> - spin_unlock(&group->notification_lock);
> + finish_permission_event(group, event, response);
> wake_up(&group->fanotify_data.access_waitq);
> return 0;
> }
> @@ -302,7 +319,9 @@ static ssize_t fanotify_read(struct file *file, char __user *buf,
> fsnotify_destroy_event(group, kevent);
> } else {
> if (ret <= 0) {
> - FANOTIFY_PE(kevent)->response = FAN_DENY;
> + spin_lock(&group->notification_lock);
> + finish_permission_event(group,
> + FANOTIFY_PE(kevent), FAN_DENY);
> wake_up(&group->fanotify_data.access_waitq);
> } else {
> spin_lock(&group->notification_lock);
> @@ -371,7 +390,8 @@ static int fanotify_release(struct inode *ignored, struct file *file)
> event = list_first_entry(&group->fanotify_data.access_list,
> struct fanotify_perm_event_info, fae.fse.list);
> list_del_init(&event->fae.fse.list);
> - event->response = FAN_ALLOW;
> + finish_permission_event(group, event, FAN_ALLOW);
> + spin_lock(&group->notification_lock);
> }
>
> /*
> @@ -384,10 +404,11 @@ static int fanotify_release(struct inode *ignored, struct file *file)
> if (!(fsn_event->mask & FANOTIFY_PERM_EVENTS)) {
> spin_unlock(&group->notification_lock);
> fsnotify_destroy_event(group, fsn_event);
> - spin_lock(&group->notification_lock);
> } else {
> - FANOTIFY_PE(fsn_event)->response = FAN_ALLOW;
> + finish_permission_event(group, FANOTIFY_PE(fsn_event),
> + FAN_ALLOW);
> }
> + spin_lock(&group->notification_lock);
> }
> spin_unlock(&group->notification_lock);
>
> --
> 2.16.4
>
