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>
This commit is contained in:
parent
13116dfd13
commit
83c0e1b442
|
@ -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
|
||||
|
|
|
@ -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)
|
||||
|
|
|
@ -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;
|
||||
}
|
||||
|
||||
/*
|
||||
|
|
|
@ -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 */
|
||||
|
|
Loading…
Reference in New Issue