Skip to content

Commit

Permalink
fsnotify: Fix NULL ptr deref in fanotify_get_fsid()
Browse files Browse the repository at this point in the history
fanotify_get_fsid() is reading mark->connector->fsid under srcu. It can
happen that it sees mark not fully initialized or mark that is already
detached from the object list. In these cases mark->connector
can be NULL leading to NULL ptr dereference. Fix the problem by
being careful when reading mark->connector and check it for being NULL.
Also use WRITE_ONCE when writing the mark just to prevent compiler from
doing something stupid.

Reported-by: syzbot+15927486a4f1bfcbaf91@syzkaller.appspotmail.com
Fixes: 7711522 ("fanotify: cache fsid in fsnotify_mark_connector")
Signed-off-by: Jan Kara <jack@suse.cz>
  • Loading branch information
Jan Kara committed Apr 28, 2019
1 parent ba25b50 commit b1da6a5
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 8 deletions.
14 changes: 12 additions & 2 deletions fs/notify/fanotify/fanotify.c
Original file line number Diff line number Diff line change
Expand Up @@ -346,10 +346,16 @@ static __kernel_fsid_t fanotify_get_fsid(struct fsnotify_iter_info *iter_info)
__kernel_fsid_t fsid = {};

fsnotify_foreach_obj_type(type) {
struct fsnotify_mark_connector *conn;

if (!fsnotify_iter_should_report_type(iter_info, type))
continue;

fsid = iter_info->marks[type]->connector->fsid;
conn = READ_ONCE(iter_info->marks[type]->connector);
/* Mark is just getting destroyed or created? */
if (!conn)
continue;
fsid = conn->fsid;
if (WARN_ON_ONCE(!fsid.val[0] && !fsid.val[1]))
continue;
return fsid;
Expand Down Expand Up @@ -408,8 +414,12 @@ static int fanotify_handle_event(struct fsnotify_group *group,
return 0;
}

if (FAN_GROUP_FLAG(group, FAN_REPORT_FID))
if (FAN_GROUP_FLAG(group, FAN_REPORT_FID)) {
fsid = fanotify_get_fsid(iter_info);
/* Racing with mark destruction or creation? */
if (!fsid.val[0] && !fsid.val[1])
return 0;
}

event = fanotify_alloc_event(group, inode, mask, data, data_type,
&fsid);
Expand Down
12 changes: 6 additions & 6 deletions fs/notify/mark.c
Original file line number Diff line number Diff line change
Expand Up @@ -239,13 +239,13 @@ static void fsnotify_drop_object(unsigned int type, void *objp)

void fsnotify_put_mark(struct fsnotify_mark *mark)
{
struct fsnotify_mark_connector *conn;
struct fsnotify_mark_connector *conn = READ_ONCE(mark->connector);
void *objp = NULL;
unsigned int type = FSNOTIFY_OBJ_TYPE_DETACHED;
bool free_conn = false;

/* Catch marks that were actually never attached to object */
if (!mark->connector) {
if (!conn) {
if (refcount_dec_and_test(&mark->refcnt))
fsnotify_final_mark_destroy(mark);
return;
Expand All @@ -255,18 +255,17 @@ void fsnotify_put_mark(struct fsnotify_mark *mark)
* We have to be careful so that traversals of obj_list under lock can
* safely grab mark reference.
*/
if (!refcount_dec_and_lock(&mark->refcnt, &mark->connector->lock))
if (!refcount_dec_and_lock(&mark->refcnt, &conn->lock))
return;

conn = mark->connector;
hlist_del_init_rcu(&mark->obj_list);
if (hlist_empty(&conn->list)) {
objp = fsnotify_detach_connector_from_object(conn, &type);
free_conn = true;
} else {
__fsnotify_recalc_mask(conn);
}
mark->connector = NULL;
WRITE_ONCE(mark->connector, NULL);
spin_unlock(&conn->lock);

fsnotify_drop_object(type, objp);
Expand Down Expand Up @@ -620,7 +619,7 @@ static int fsnotify_add_mark_list(struct fsnotify_mark *mark,
/* mark should be the last entry. last is the current last entry */
hlist_add_behind_rcu(&mark->obj_list, &last->obj_list);
added:
mark->connector = conn;
WRITE_ONCE(mark->connector, conn);
out_err:
spin_unlock(&conn->lock);
spin_unlock(&mark->lock);
Expand Down Expand Up @@ -808,6 +807,7 @@ void fsnotify_init_mark(struct fsnotify_mark *mark,
refcount_set(&mark->refcnt, 1);
fsnotify_get_group(group);
mark->group = group;
WRITE_ONCE(mark->connector, NULL);
}

/*
Expand Down

0 comments on commit b1da6a5

Please sign in to comment.