Commit 1ecd7450 authored by Linus Torvalds's avatar Linus Torvalds

Merge branch 'for_linus' of git://git.kernel.org/pub/scm/linux/kernel/git/jack/linux-fs

Pull fanotify use-after-free fixes from Jan Kara:
 "Three fixes for the fanotify use after free problems guys were
  reporting.

  I have ended up with different lifetime rules for struct
  fanotify_event_info depending on whether it is for permission event or
  normal event which isn't ideal.  My plan is to split these into two
  different structures (as permission events need larger struct anyway)
  which will make the rules trivial again.  But that can wait for later
  I guess (but I can add the patch to the pile if you want), now I
  wanted to make -rc1 boot for these guys"

[ "These guys" being Jiri Kosina and Dave Jones that reported the slab
  corruption issues due to incorrect object lifetimes ]

* 'for_linus' of git://git.kernel.org/pub/scm/linux/kernel/git/jack/linux-fs:
  fanotify: Fix use after free for permission events
  fsnotify: Do not return merged event from fsnotify_add_notify_event()
  fanotify: Fix use after free in mask checking
parents 72466d0b 85816794
...@@ -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);
...@@ -34,14 +28,23 @@ static bool should_merge(struct fsnotify_event *old_fsn, ...@@ -34,14 +28,23 @@ static bool should_merge(struct fsnotify_event *old_fsn,
} }
/* and the list better be locked by something too! */ /* and the list better be locked by something too! */
static struct fsnotify_event *fanotify_merge(struct list_head *list, static int fanotify_merge(struct list_head *list, struct fsnotify_event *event)
struct fsnotify_event *event)
{ {
struct fsnotify_event *test_event; struct fsnotify_event *test_event;
bool do_merge = false; bool do_merge = false;
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 0;
#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;
...@@ -50,10 +53,10 @@ static struct fsnotify_event *fanotify_merge(struct list_head *list, ...@@ -50,10 +53,10 @@ static struct fsnotify_event *fanotify_merge(struct list_head *list,
} }
if (!do_merge) if (!do_merge)
return NULL; return 0;
test_event->mask |= event->mask; test_event->mask |= event->mask;
return test_event; return 1;
} }
#ifdef CONFIG_FANOTIFY_ACCESS_PERMISSIONS #ifdef CONFIG_FANOTIFY_ACCESS_PERMISSIONS
...@@ -149,7 +152,6 @@ static int fanotify_handle_event(struct fsnotify_group *group, ...@@ -149,7 +152,6 @@ static int fanotify_handle_event(struct fsnotify_group *group,
int ret = 0; int ret = 0;
struct fanotify_event_info *event; struct fanotify_event_info *event;
struct fsnotify_event *fsn_event; struct fsnotify_event *fsn_event;
struct fsnotify_event *notify_fsn_event;
BUILD_BUG_ON(FAN_ACCESS != FS_ACCESS); BUILD_BUG_ON(FAN_ACCESS != FS_ACCESS);
BUILD_BUG_ON(FAN_MODIFY != FS_MODIFY); BUILD_BUG_ON(FAN_MODIFY != FS_MODIFY);
...@@ -188,21 +190,19 @@ static int fanotify_handle_event(struct fsnotify_group *group, ...@@ -188,21 +190,19 @@ static int fanotify_handle_event(struct fsnotify_group *group,
event->response = 0; event->response = 0;
#endif #endif
notify_fsn_event = fsnotify_add_notify_event(group, fsn_event, ret = fsnotify_add_notify_event(group, fsn_event, fanotify_merge);
fanotify_merge); if (ret) {
if (notify_fsn_event) { BUG_ON(mask & FAN_ALL_PERM_EVENTS);
/* Our event wasn't used in the end. Free it. */ /* Our event wasn't used in the end. Free it. */
fsnotify_destroy_event(group, fsn_event); fsnotify_destroy_event(group, fsn_event);
if (IS_ERR(notify_fsn_event)) ret = 0;
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);
fsnotify_destroy_event(group, fsn_event);
}
#endif #endif
return ret; return ret;
} }
......
...@@ -4,6 +4,13 @@ ...@@ -4,6 +4,13 @@
extern struct kmem_cache *fanotify_event_cachep; extern struct kmem_cache *fanotify_event_cachep;
/*
* Lifetime of the structure differs for normal and permission events. In both
* cases the structure is allocated in fanotify_handle_event(). For normal
* events the structure is freed immediately after reporting it to userspace.
* For permission events we free it only after we receive response from
* userspace.
*/
struct fanotify_event_info { struct fanotify_event_info {
struct fsnotify_event fse; struct fsnotify_event fse;
/* /*
......
...@@ -319,7 +319,12 @@ static ssize_t fanotify_read(struct file *file, char __user *buf, ...@@ -319,7 +319,12 @@ static ssize_t fanotify_read(struct file *file, char __user *buf,
if (IS_ERR(kevent)) if (IS_ERR(kevent))
break; break;
ret = copy_event_to_user(group, kevent, buf); ret = copy_event_to_user(group, kevent, buf);
fsnotify_destroy_event(group, kevent); /*
* Permission events get destroyed after we
* receive response
*/
if (!(kevent->mask & FAN_ALL_PERM_EVENTS))
fsnotify_destroy_event(group, kevent);
if (ret < 0) if (ret < 0)
break; break;
buf += ret; buf += ret;
......
...@@ -53,15 +53,13 @@ static bool event_compare(struct fsnotify_event *old_fsn, ...@@ -53,15 +53,13 @@ static bool event_compare(struct fsnotify_event *old_fsn,
return false; return false;
} }
static struct fsnotify_event *inotify_merge(struct list_head *list, static int inotify_merge(struct list_head *list,
struct fsnotify_event *event) struct fsnotify_event *event)
{ {
struct fsnotify_event *last_event; struct fsnotify_event *last_event;
last_event = list_entry(list->prev, struct fsnotify_event, list); last_event = list_entry(list->prev, struct fsnotify_event, list);
if (!event_compare(last_event, event)) return event_compare(last_event, event);
return NULL;
return last_event;
} }
int inotify_handle_event(struct fsnotify_group *group, int inotify_handle_event(struct fsnotify_group *group,
...@@ -73,9 +71,8 @@ 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_inode_mark *i_mark;
struct inotify_event_info *event; struct inotify_event_info *event;
struct fsnotify_event *added_event;
struct fsnotify_event *fsn_event; struct fsnotify_event *fsn_event;
int ret = 0; int ret;
int len = 0; int len = 0;
int alloc_len = sizeof(struct inotify_event_info); int alloc_len = sizeof(struct inotify_event_info);
...@@ -110,18 +107,16 @@ int inotify_handle_event(struct fsnotify_group *group, ...@@ -110,18 +107,16 @@ int inotify_handle_event(struct fsnotify_group *group,
if (len) if (len)
strcpy(event->name, file_name); strcpy(event->name, file_name);
added_event = fsnotify_add_notify_event(group, fsn_event, inotify_merge); ret = fsnotify_add_notify_event(group, fsn_event, inotify_merge);
if (added_event) { if (ret) {
/* Our event wasn't used in the end. Free it. */ /* Our event wasn't used in the end. Free it. */
fsnotify_destroy_event(group, fsn_event); fsnotify_destroy_event(group, fsn_event);
if (IS_ERR(added_event))
ret = PTR_ERR(added_event);
} }
if (inode_mark->mask & IN_ONESHOT) if (inode_mark->mask & IN_ONESHOT)
fsnotify_destroy_mark(inode_mark, group); fsnotify_destroy_mark(inode_mark, group);
return ret; return 0;
} }
static void inotify_freeing_mark(struct fsnotify_mark *fsn_mark, struct fsnotify_group *group) 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, ...@@ -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 * 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 * event off the queue to deal with. The function returns 0 if the event was
* group's notification queue, a reference is taken on event. * 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, int fsnotify_add_notify_event(struct fsnotify_group *group,
struct fsnotify_event *event, struct fsnotify_event *event,
struct fsnotify_event *(*merge)(struct list_head *, int (*merge)(struct list_head *,
struct fsnotify_event *)) struct fsnotify_event *))
{ {
struct fsnotify_event *return_event = NULL; int ret = 0;
struct list_head *list = &group->notification_list; struct list_head *list = &group->notification_list;
pr_debug("%s: group=%p event=%p\n", __func__, group, event); 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, ...@@ -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 */ /* Queue overflow event only if it isn't already queued */
if (list_empty(&group->overflow_event.list)) if (list_empty(&group->overflow_event.list))
event = &group->overflow_event; event = &group->overflow_event;
return_event = event; ret = 1;
} }
if (!list_empty(list) && merge) { if (!list_empty(list) && merge) {
return_event = merge(list, event); ret = merge(list, event);
if (return_event) { if (ret) {
mutex_unlock(&group->notification_mutex); 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, ...@@ -115,7 +115,7 @@ struct fsnotify_event *fsnotify_add_notify_event(struct fsnotify_group *group,
wake_up(&group->notification_waitq); wake_up(&group->notification_waitq);
kill_fasync(&group->fsn_fa, SIGIO, POLL_IN); 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); ...@@ -322,10 +322,10 @@ extern int fsnotify_fasync(int fd, struct file *file, int on);
extern void fsnotify_destroy_event(struct fsnotify_group *group, extern void fsnotify_destroy_event(struct fsnotify_group *group,
struct fsnotify_event *event); struct fsnotify_event *event);
/* attach the event to the group notification queue */ /* attach the event to the group notification queue */
extern struct fsnotify_event *fsnotify_add_notify_event(struct fsnotify_group *group, extern int fsnotify_add_notify_event(struct fsnotify_group *group,
struct fsnotify_event *event, struct fsnotify_event *event,
struct fsnotify_event *(*merge)(struct list_head *, int (*merge)(struct list_head *,
struct fsnotify_event *)); struct fsnotify_event *));
/* true if the group notification queue is empty */ /* true if the group notification queue is empty */
extern bool fsnotify_notify_queue_is_empty(struct fsnotify_group *group); extern bool fsnotify_notify_queue_is_empty(struct fsnotify_group *group);
/* return, but do not dequeue the first event on the notification queue */ /* return, but do not dequeue the first event on the notification queue */
......
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