[5/6] fanotify: Track permission event state
diff mbox series

Message ID 20190213145443.26836-6-jack@suse.cz
State New
Headers show
Series
  • fanotify: Make wait for permission event response interruptible
Related show

Commit Message

Jan Kara Feb. 13, 2019, 2:54 p.m. UTC
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@suse.cz>
---
 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(-)

Comments

Amir Goldstein Feb. 13, 2019, 8:33 p.m. UTC | #1
On Wed, Feb 13, 2019 at 4:54 PM Jan Kara <jack@suse.cz> 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@suse.cz>
Reviewed-by: Amir Goldstein <amir73il@gmail.com>

> ---
>  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
>

Patch
diff mbox series

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);