@@ -349,7 +349,7 @@ static __kernel_fsid_t fanotify_get_fsid(struct fsnotify_iter_info *iter_info)
if (!fsnotify_iter_should_report_type(iter_info, type))
continue;
- fsid = iter_info->marks[type]->connector->fsid;
+ fsid = READ_ONCE(iter_info->marks[type]->connector)->fsid;
if (WARN_ON_ONCE(!fsid.val[0] && !fsid.val[1]))
continue;
return fsid;
@@ -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;
@@ -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
* 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);
@@ -543,6 +542,23 @@ static struct fsnotify_mark_connector *fsnotify_grab_connector(
return conn;
}
+/*
+ * We need to assign mark->connector before adding mark to the list so that
+ * RCU readers can safely dereference it but do not want to set it before
+ * we are really sure we are adding the mark to the connector's list so that
+ * someone cannot see connector value that was reset back to NULL in the end.
+ */
+#define fanotify_attach_obj_list(where, mark, conn, head) \
+ do {\
+ mark->connector = conn;\
+ /*
+ * Make sure RCU readers of object list see connector
+ * initialized. Pairs in READ_ONCE for unlocked readers.
+ */\
+ smp_wmb();\
+ hlist_add_##where##_rcu(&mark->obj_list, head);\
+ } while (0)
+
/*
* Add mark into proper place in given list of marks. These marks may be used
* for the fsnotify backend to determine which event types should be delivered
@@ -589,13 +605,13 @@ static int fsnotify_add_mark_list(struct fsnotify_mark *mark,
fsid->val[0], fsid->val[1],
conn->fsid.val[0], conn->fsid.val[1]);
err = -EXDEV;
- goto out_err;
+ goto out;
}
/* is mark the first mark? */
if (hlist_empty(&conn->list)) {
- hlist_add_head_rcu(&mark->obj_list, &conn->list);
- goto added;
+ fanotify_attach_obj_list(head, mark, conn, &conn->list);
+ goto out;
}
/* should mark be in the middle of the current list? */
@@ -606,22 +622,21 @@ static int fsnotify_add_mark_list(struct fsnotify_mark *mark,
(lmark->flags & FSNOTIFY_MARK_FLAG_ATTACHED) &&
!allow_dups) {
err = -EEXIST;
- goto out_err;
+ goto out;
}
cmp = fsnotify_compare_groups(lmark->group, mark->group);
if (cmp >= 0) {
- hlist_add_before_rcu(&mark->obj_list, &lmark->obj_list);
- goto added;
+ fanotify_attach_obj_list(before, mark, conn,
+ &lmark->obj_list);
+ goto out;
}
}
BUG_ON(last == NULL);
/* 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;
-out_err:
+ fanotify_attach_obj_list(behind, mark, conn, &last->obj_list);
+out:
spin_unlock(&conn->lock);
spin_unlock(&mark->lock);
return err;
fanotify_get_fsid() is reading mark->connector->fsid under srcu. It can happen that it sees mark not fully initialized and thus mark->connector is still NULL leading to NULL ptr dereference. Fix the problem by setting mark->connector before adding mark to the object's list and use appropriate memory barriers to make sure proper mark->connector value is seen for any mark added to the list. Reported-by: syzbot+15927486a4f1bfcbaf91@syzkaller.appspotmail.com Fixes: 77115225acc6 ("fanotify: cache fsid in fsnotify_mark_connector") Signed-off-by: Jan Kara <jack@suse.cz> --- fs/notify/fanotify/fanotify.c | 2 +- fs/notify/mark.c | 43 +++++++++++++++++++++++++++++-------------- 2 files changed, 30 insertions(+), 15 deletions(-)