fsnotify: Fix NULL ptr deref in fanotify_get_fsid() [Linux 5.1]

fsnotify: Fix NULL ptr deref in fanotify_get_fsid() [Linux 5.1]

This Linux kernel change "fsnotify: Fix NULL ptr deref in fanotify_get_fsid()" is included in the Linux 5.1 release. This change is authored by Jan Kara <jack [at] suse.cz> on Wed Apr 24 18:39:57 2019 +0200. The commit for this change in Linux stable tree is b1da6a5 (patch).

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: [email protected]
Fixes: 77115225acc6 ("fanotify: cache fsid in fsnotify_mark_connector")
Signed-off-by: Jan Kara <[email protected]>

There are 26 lines of Linux source code added/deleted in this change. Code changes to Linux kernel are as follows.

 fs/notify/fanotify/fanotify.c | 14 ++++++++++++--
 fs/notify/mark.c              | 12 ++++++------
 2 files changed, 18 insertions(+), 8 deletions(-)

diff --git a/fs/notify/fanotify/fanotify.c b/fs/notify/fanotify/fanotify.c
index 6b9c275..63c6bb1 100644
--- a/fs/notify/fanotify/fanotify.c
+++ b/fs/notify/fanotify/fanotify.c
@@ -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;
@@ -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);
diff --git a/fs/notify/mark.c b/fs/notify/mark.c
index d593d42..22acb0a 100644
--- a/fs/notify/mark.c
+++ b/fs/notify/mark.c
@@ -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);
@@ -266,7 +265,7 @@ void fsnotify_put_mark(struct fsnotify_mark *mark)
    } else {
        __fsnotify_recalc_mask(conn);
    }
-   mark->connector = NULL;
+   WRITE_ONCE(mark->connector, NULL);
    spin_unlock(&conn->lock);

    fsnotify_drop_object(type, objp);
@@ -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);
@@ -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);
 }

 /*

Leave a Reply

Your email address will not be published. Required fields are marked *