Commit b1da6a51 authored by Jan Kara's avatar Jan Kara
Browse files

fsnotify: Fix NULL ptr deref in fanotify_get_fsid() 



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: default avatar <syzbot+15927486a4f1bfcbaf91@syzkaller.appspotmail.com>
Fixes: 77115225 ("fanotify: cache fsid in fsnotify_mark_connector")
Signed-off-by: default avatarJan Kara <jack@suse.cz>
parent ba25b50d

fs/notify/fanotify/fanotify.c

+12 −2
Original line number Original line Diff line number Diff line
@@ -346,10 +346,16 @@ static __kernel_fsid_t fanotify_get_fsid(struct fsnotify_iter_info *iter_info) 
	__kernel_fsid_t fsid = {};

	__kernel_fsid_t fsid = {};





	fsnotify_foreach_obj_type(type) {

	fsnotify_foreach_obj_type(type) {

		struct fsnotify_mark_connector *conn;



		if (!fsnotify_iter_should_report_type(iter_info, type))

		if (!fsnotify_iter_should_report_type(iter_info, type))

			continue;

			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]))

		if (WARN_ON_ONCE(!fsid.val[0] && !fsid.val[1]))

			continue;

			continue;

		return fsid;

		return fsid;
@@ -408,8 +414,12 @@ static int fanotify_handle_event(struct fsnotify_group *group, 
			return 0;

			return 0;

	}

	}





	if (FAN_GROUP_FLAG(group, FAN_REPORT_FID))

	if (FAN_GROUP_FLAG(group, FAN_REPORT_FID)) {

		fsid = fanotify_get_fsid(iter_info);

		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,

	event = fanotify_alloc_event(group, inode, mask, data, data_type,

				     &fsid);

				     &fsid);

fs/notify/mark.c

+6 −6
Original line number Original line Diff line number Diff line
@@ -239,13 +239,13 @@ static void fsnotify_drop_object(unsigned int type, void *objp) 




void fsnotify_put_mark(struct fsnotify_mark *mark)

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;

	void *objp = NULL;

	unsigned int type = FSNOTIFY_OBJ_TYPE_DETACHED;

	unsigned int type = FSNOTIFY_OBJ_TYPE_DETACHED;

	bool free_conn = false;

	bool free_conn = false;





	/* Catch marks that were actually never attached to object */

	/* Catch marks that were actually never attached to object */

	if (!mark->connector) {

	if (!conn) {

		if (refcount_dec_and_test(&mark->refcnt))

		if (refcount_dec_and_test(&mark->refcnt))

			fsnotify_final_mark_destroy(mark);

			fsnotify_final_mark_destroy(mark);

		return;

		return;
@@ -255,10 +255,9 @@ void fsnotify_put_mark(struct fsnotify_mark *mark) 
	 * We have to be careful so that traversals of obj_list under lock can

	 * We have to be careful so that traversals of obj_list under lock can

	 * safely grab mark reference.

	 * safely grab mark reference.

	 */

	 */

	if (!refcount_dec_and_lock(&mark->refcnt, &mark->connector->lock))

	if (!refcount_dec_and_lock(&mark->refcnt, &conn->lock))

		return;

		return;





	conn = mark->connector;

	hlist_del_init_rcu(&mark->obj_list);

	hlist_del_init_rcu(&mark->obj_list);

	if (hlist_empty(&conn->list)) {

	if (hlist_empty(&conn->list)) {

		objp = fsnotify_detach_connector_from_object(conn, &type);

		objp = fsnotify_detach_connector_from_object(conn, &type);
@@ -266,7 +265,7 @@ void fsnotify_put_mark(struct fsnotify_mark *mark) 
	} else {

	} else {

		__fsnotify_recalc_mask(conn);

		__fsnotify_recalc_mask(conn);

	}

	}

	mark->connector = NULL;

	WRITE_ONCE(mark->connector, NULL);

	spin_unlock(&conn->lock);

	spin_unlock(&conn->lock);





	fsnotify_drop_object(type, objp);

	fsnotify_drop_object(type, objp);
@@ -620,7 +619,7 @@ restart: 
	/* mark should be the last entry.  last is the current last entry */

	/* mark should be the last entry.  last is the current last entry */

	hlist_add_behind_rcu(&mark->obj_list, &last->obj_list);

	hlist_add_behind_rcu(&mark->obj_list, &last->obj_list);

added:

added:

	mark->connector = conn;

	WRITE_ONCE(mark->connector, conn);

out_err:

out_err:

	spin_unlock(&conn->lock);

	spin_unlock(&conn->lock);

	spin_unlock(&mark->lock);

	spin_unlock(&mark->lock);
@@ -808,6 +807,7 @@ void fsnotify_init_mark(struct fsnotify_mark *mark, 
	refcount_set(&mark->refcnt, 1);

	refcount_set(&mark->refcnt, 1);

	fsnotify_get_group(group);

	fsnotify_get_group(group);

	mark->group = group;

	mark->group = group;

	WRITE_ONCE(mark->connector, NULL);

}

}





/*

/*

CRA Git | Maintained and supported by SUSTech CRA and CCSE