On Tue, Jan 8, 2019 at 6:46 PM Jan Kara <jack@xxxxxxx> wrote:
>
> Track whether permission event got already reported to userspace and
> whether userspace already answered to the permission event in high bits
> of its ->response field. Also protect stores 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>
> ---
> fs/notify/fanotify/fanotify.c | 16 ++++++++++------
> fs/notify/fanotify/fanotify.h | 10 +++++++++-
> fs/notify/fanotify/fanotify_user.c | 17 ++++++++++++-----
> 3 files changed, 31 insertions(+), 12 deletions(-)
>
> diff --git a/fs/notify/fanotify/fanotify.c b/fs/notify/fanotify/fanotify.c
> index 3723f3d18d20..cca13adc3a4c 100644
> --- a/fs/notify/fanotify/fanotify.c
> +++ b/fs/notify/fanotify/fanotify.c
> @@ -62,13 +62,19 @@ static int fanotify_get_response(struct fsnotify_group *group,
> struct fsnotify_iter_info *iter_info)
> {
> int ret;
> + unsigned int response;
>
> + BUILD_BUG_ON(FAN_EVENT_STATE_MASK & (FAN_AUDIT | FAN_ALLOW | FAN_DENY));
> 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->response & FAN_EVENT_STATE_MASK) ==
> + FAN_EVENT_ANSWERED);
> +
> + response = event->response & ~FAN_EVENT_STATE_MASK;
>
> /* userspace responded, convert to something usable */
> - switch (event->response & ~FAN_AUDIT) {
> + switch (response & ~FAN_AUDIT) {
> case FAN_ALLOW:
> ret = 0;
> break;
> @@ -78,10 +84,8 @@ static int fanotify_get_response(struct fsnotify_group *group,
> }
>
> /* Check if the response should be audited */
> - if (event->response & FAN_AUDIT)
> - audit_fanotify(event->response & ~FAN_AUDIT);
> -
> - event->response = 0;
> + if (response & FAN_AUDIT)
> + audit_fanotify(response & ~FAN_AUDIT);
>
> pr_debug("%s: group=%p event=%p about to return ret=%d\n", __func__,
> group, event, ret);
> diff --git a/fs/notify/fanotify/fanotify.h b/fs/notify/fanotify/fanotify.h
> index ea05b8a401e7..954d997745c3 100644
> --- a/fs/notify/fanotify/fanotify.h
> +++ b/fs/notify/fanotify/fanotify.h
> @@ -22,6 +22,12 @@ struct fanotify_event_info {
> struct pid *pid;
> };
>
> +/* State of permission event we store inside response field */
> +#define FAN_EVENT_STATE_MASK 0xc0000000
> +
> +#define FAN_EVENT_REPORTED 0x40000000 /* Event reported to userspace */
> +#define FAN_EVENT_ANSWERED 0x80000000 /* Event answered by userspace */
> +
> /*
> * 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 +37,9 @@ struct fanotify_event_info {
> */
> struct fanotify_perm_event_info {
> struct fanotify_event_info fae;
> - int response; /* userspace answer to question */
> + unsigned int response; /* userspace answer to the event. We also use
> + * high bits of this field for recording state
> + * of the event. */
What's wrong with:
short response; /* userspace answer to question */
short state; /* the state of this event */
Will make for a much simpler code/patch IMO.
Thanks,
Amir.
