Commit 13116dfd authored by Jan Kara's avatar Jan Kara

fanotify: Fix use after free in mask checking

We cannot use the event structure returned from
fsnotify_add_notify_event() because that event can be freed by the time
that function returns. Use the mask argument passed into the event
handler directly instead. This also fixes a possible problem when we
could unnecessarily wait for permission response for a normal fanotify
event which got merged with a permission event.

We also disallow merging of permission event with any other event so
that we know the permission event which we just created is the one on
which we should wait for permission response.
Reported-and-tested-by: default avatarJiri Kosina <jkosina@suse.cz>
Reported-and-tested-by: default avatarDave Jones <davej@fedoraproject.org>
Signed-off-by: default avatarJan Kara <jack@suse.cz>
parent 0e47c969
...@@ -16,12 +16,6 @@ static bool should_merge(struct fsnotify_event *old_fsn, ...@@ -16,12 +16,6 @@ static bool should_merge(struct fsnotify_event *old_fsn,
{ {
struct fanotify_event_info *old, *new; struct fanotify_event_info *old, *new;
#ifdef CONFIG_FANOTIFY_ACCESS_PERMISSIONS
/* dont merge two permission events */
if ((old_fsn->mask & FAN_ALL_PERM_EVENTS) &&
(new_fsn->mask & FAN_ALL_PERM_EVENTS))
return false;
#endif
pr_debug("%s: old=%p new=%p\n", __func__, old_fsn, new_fsn); pr_debug("%s: old=%p new=%p\n", __func__, old_fsn, new_fsn);
old = FANOTIFY_E(old_fsn); old = FANOTIFY_E(old_fsn);
new = FANOTIFY_E(new_fsn); new = FANOTIFY_E(new_fsn);
...@@ -42,6 +36,16 @@ static struct fsnotify_event *fanotify_merge(struct list_head *list, ...@@ -42,6 +36,16 @@ static struct fsnotify_event *fanotify_merge(struct list_head *list,
pr_debug("%s: list=%p event=%p\n", __func__, list, event); pr_debug("%s: list=%p event=%p\n", __func__, list, event);
#ifdef CONFIG_FANOTIFY_ACCESS_PERMISSIONS
/*
* Don't merge a permission event with any other event so that we know
* the event structure we have created in fanotify_handle_event() is the
* one we should check for permission response.
*/
if (event->mask & FAN_ALL_PERM_EVENTS)
return NULL;
#endif
list_for_each_entry_reverse(test_event, list, list) { list_for_each_entry_reverse(test_event, list, list) {
if (should_merge(test_event, event)) { if (should_merge(test_event, event)) {
do_merge = true; do_merge = true;
...@@ -195,13 +199,10 @@ static int fanotify_handle_event(struct fsnotify_group *group, ...@@ -195,13 +199,10 @@ static int fanotify_handle_event(struct fsnotify_group *group,
fsnotify_destroy_event(group, fsn_event); fsnotify_destroy_event(group, fsn_event);
if (IS_ERR(notify_fsn_event)) if (IS_ERR(notify_fsn_event))
return PTR_ERR(notify_fsn_event); return PTR_ERR(notify_fsn_event);
/* We need to ask about a different events after a merge... */
event = FANOTIFY_E(notify_fsn_event);
fsn_event = notify_fsn_event;
} }
#ifdef CONFIG_FANOTIFY_ACCESS_PERMISSIONS #ifdef CONFIG_FANOTIFY_ACCESS_PERMISSIONS
if (fsn_event->mask & FAN_ALL_PERM_EVENTS) if (mask & FAN_ALL_PERM_EVENTS)
ret = fanotify_get_response_from_access(group, event); ret = fanotify_get_response_from_access(group, event);
#endif #endif
return ret; return ret;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment