fsnotify: Do not return merged event from fsnotify_add_notify_event()

The event returned from fsnotify_add_notify_event() cannot ever be used
safely as the event may be freed by the time the function returns (after
dropping notification_mutex). So change the prototype to just return
whether the event was added or merged into some existing event.

Reported-and-tested-by: Jiri Kosina <jkosina@suse.cz>
Reported-and-tested-by: Dave Jones <davej@fedoraproject.org>
Signed-off-by: Jan Kara <jack@suse.cz>

4 files changed, 30 insertions, 39 deletions

diff --git a/fs/notify/fanotify/fanotify.c b/fs/notify/fanotify/fanotify.c
index cc78e2fbc8e4..c7e5e8f54748 100644
--- a/fs/notify/fanotify/fanotify.c
+++ b/fs/notify/fanotify/fanotify.c
@@ -28,8 +28,7 @@ static bool should_merge(struct fsnotify_event *old_fsn,
 }
 
 /* and the list better be locked by something too! */
-static struct fsnotify_event *fanotify_merge(struct list_head *list,
-                                             struct fsnotify_event *event)
+static int fanotify_merge(struct list_head *list, struct fsnotify_event *event)
 {
         struct fsnotify_event *test_event;
         bool do_merge = false;
@@ -43,7 +42,7 @@ static struct fsnotify_event *fanotify_merge(struct list_head *list,
          * one we should check for permission response.
          */
         if (event->mask & FAN_ALL_PERM_EVENTS)
-                return NULL;
+                return 0;
 #endif
 
         list_for_each_entry_reverse(test_event, list, list) {
@@ -54,10 +53,10 @@ static struct fsnotify_event *fanotify_merge(struct list_head *list,
         }
 
         if (!do_merge)
-                return NULL;
+                return 0;
 
         test_event->mask |= event->mask;
-        return test_event;
+        return 1;
 }
 
 #ifdef CONFIG_FANOTIFY_ACCESS_PERMISSIONS
@@ -153,7 +152,6 @@ static int fanotify_handle_event(struct fsnotify_group *group,
         int ret = 0;
         struct fanotify_event_info *event;
         struct fsnotify_event *fsn_event;
-        struct fsnotify_event *notify_fsn_event;
 
         BUILD_BUG_ON(FAN_ACCESS != FS_ACCESS);
         BUILD_BUG_ON(FAN_MODIFY != FS_MODIFY);
@@ -192,13 +190,11 @@ static int fanotify_handle_event(struct fsnotify_group *group,
         event->response = 0;
 #endif
 
-        notify_fsn_event = fsnotify_add_notify_event(group, fsn_event,
-                                                     fanotify_merge);
-        if (notify_fsn_event) {
+        ret = fsnotify_add_notify_event(group, fsn_event, fanotify_merge);
+        if (ret) {
                 /* Our event wasn't used in the end. Free it. */
                 fsnotify_destroy_event(group, fsn_event);
-                if (IS_ERR(notify_fsn_event))
-                        return PTR_ERR(notify_fsn_event);
+                ret = 0;
         }
 
 #ifdef CONFIG_FANOTIFY_ACCESS_PERMISSIONS
diff --git a/fs/notify/inotify/inotify_fsnotify.c b/fs/notify/inotify/inotify_fsnotify.c
index aad1a35e9af1..d5ee56348bb8 100644
--- a/fs/notify/inotify/inotify_fsnotify.c
+++ b/fs/notify/inotify/inotify_fsnotify.c
@@ -53,15 +53,13 @@ static bool event_compare(struct fsnotify_event *old_fsn,
         return false;
 }
 
-static struct fsnotify_event *inotify_merge(struct list_head *list,
-                                            struct fsnotify_event *event)
+static int inotify_merge(struct list_head *list,
+                          struct fsnotify_event *event)
 {
         struct fsnotify_event *last_event;
 
         last_event = list_entry(list->prev, struct fsnotify_event, list);
-        if (!event_compare(last_event, event))
-                return NULL;
-        return last_event;
+        return event_compare(last_event, event);
 }
 
 int inotify_handle_event(struct fsnotify_group *group,
@@ -73,9 +71,8 @@ int inotify_handle_event(struct fsnotify_group *group,
 {
         struct inotify_inode_mark *i_mark;
         struct inotify_event_info *event;
-        struct fsnotify_event *added_event;
         struct fsnotify_event *fsn_event;
-        int ret = 0;
+        int ret;
         int len = 0;
         int alloc_len = sizeof(struct inotify_event_info);
 
@@ -110,18 +107,16 @@ int inotify_handle_event(struct fsnotify_group *group,
         if (len)
                 strcpy(event->name, file_name);
 
-        added_event = fsnotify_add_notify_event(group, fsn_event, inotify_merge);
-        if (added_event) {
+        ret = fsnotify_add_notify_event(group, fsn_event, inotify_merge);
+        if (ret) {
                 /* Our event wasn't used in the end. Free it. */
                 fsnotify_destroy_event(group, fsn_event);
-                if (IS_ERR(added_event))
-                        ret = PTR_ERR(added_event);
         }
 
         if (inode_mark->mask & IN_ONESHOT)
                 fsnotify_destroy_mark(inode_mark, group);
 
-        return ret;
+        return 0;
 }
 
 static void inotify_freeing_mark(struct fsnotify_mark *fsn_mark, struct fsnotify_group *group)
diff --git a/fs/notify/notification.c b/fs/notify/notification.c
index 952237b8e2d2..18b3c4427dca 100644
--- a/fs/notify/notification.c
+++ b/fs/notify/notification.c
@@ -79,15 +79,15 @@ void fsnotify_destroy_event(struct fsnotify_group *group,
 
 /*
  * Add an event to the group notification queue.  The group can later pull this
- * event off the queue to deal with.  If the event is successfully added to the
- * group's notification queue, a reference is taken on event.
+ * event off the queue to deal with.  The function returns 0 if the event was
+ * added to the queue, 1 if the event was merged with some other queued event.
  */
-struct fsnotify_event *fsnotify_add_notify_event(struct fsnotify_group *group,
-                                                 struct fsnotify_event *event,
-                                                 struct fsnotify_event *(*merge)(struct list_head *,
-                                                                                 struct fsnotify_event *))
+int fsnotify_add_notify_event(struct fsnotify_group *group,
+                              struct fsnotify_event *event,
+                              int (*merge)(struct list_head *,
+                                           struct fsnotify_event *))
 {
-        struct fsnotify_event *return_event = NULL;
+        int ret = 0;
         struct list_head *list = &group->notification_list;
 
         pr_debug("%s: group=%p event=%p\n", __func__, group, event);
@@ -98,14 +98,14 @@ struct fsnotify_event *fsnotify_add_notify_event(struct fsnotify_group *group,
                 /* Queue overflow event only if it isn't already queued */
                 if (list_empty(&group->overflow_event.list))
                         event = &group->overflow_event;
-                return_event = event;
+                ret = 1;
         }
 
         if (!list_empty(list) && merge) {
-                return_event = merge(list, event);
-                if (return_event) {
+                ret = merge(list, event);
+                if (ret) {
                         mutex_unlock(&group->notification_mutex);
-                        return return_event;
+                        return ret;
                 }
         }
 
@@ -115,7 +115,7 @@ struct fsnotify_event *fsnotify_add_notify_event(struct fsnotify_group *group,
 
         wake_up(&group->notification_waitq);
         kill_fasync(&group->fsn_fa, SIGIO, POLL_IN);
-        return return_event;
+        return ret;
 }
 
 /*
diff --git a/include/linux/fsnotify_backend.h b/include/linux/fsnotify_backend.h
index 7d8d5e608594..3d286ff49ab0 100644
--- a/include/linux/fsnotify_backend.h
+++ b/include/linux/fsnotify_backend.h
@@ -322,10 +322,10 @@ extern int fsnotify_fasync(int fd, struct file *file, int on);
 extern void fsnotify_destroy_event(struct fsnotify_group *group,
                                    struct fsnotify_event *event);
 /* attach the event to the group notification queue */
-extern struct fsnotify_event *fsnotify_add_notify_event(struct fsnotify_group *group,
-                                                        struct fsnotify_event *event,
-                                                        struct fsnotify_event *(*merge)(struct list_head *,
-                                                                                        struct fsnotify_event *));
+extern int fsnotify_add_notify_event(struct fsnotify_group *group,
+                                     struct fsnotify_event *event,
+                                     int (*merge)(struct list_head *,
+                                                  struct fsnotify_event *));
 /* true if the group notification queue is empty */
 extern bool fsnotify_notify_queue_is_empty(struct fsnotify_group *group);
 /* return, but do not dequeue the first event on the notification queue */