From patchwork Thu Apr 25 07:59:14 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jan Kara X-Patchwork-Id: 10916227 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 3DF3D1575 for ; Thu, 25 Apr 2019 07:59:21 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 32244288DA for ; Thu, 25 Apr 2019 07:59:21 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 262B028C12; Thu, 25 Apr 2019 07:59:21 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.9 required=2.0 tests=BAYES_00,MAILING_LIST_MULTI, RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 98D66288DA for ; Thu, 25 Apr 2019 07:59:20 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728665AbfDYH7T (ORCPT ); Thu, 25 Apr 2019 03:59:19 -0400 Received: from mx2.suse.de ([195.135.220.15]:41268 "EHLO mx1.suse.de" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1726380AbfDYH7T (ORCPT ); Thu, 25 Apr 2019 03:59:19 -0400 X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (unknown [195.135.220.254]) by mx1.suse.de (Postfix) with ESMTP id 6CB4EAD7B; Thu, 25 Apr 2019 07:59:17 +0000 (UTC) Received: by quack2.suse.cz (Postfix, from userid 1000) id 234711E15BE; Thu, 25 Apr 2019 09:59:17 +0200 (CEST) From: Jan Kara To: Cc: Amir Goldstein , Jan Kara Subject: [PATCH] fsnotify: Fix NULL ptr deref in fanotify_get_fsid() Date: Thu, 25 Apr 2019 09:59:14 +0200 Message-Id: <20190425075914.22030-1-jack@suse.cz> X-Mailer: git-send-email 2.16.4 Sender: linux-fsdevel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-fsdevel@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP 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 --- fs/notify/fanotify/fanotify.c | 2 +- fs/notify/mark.c | 43 +++++++++++++++++++++++++++++-------------- 2 files changed, 30 insertions(+), 15 deletions(-) diff --git a/fs/notify/fanotify/fanotify.c b/fs/notify/fanotify/fanotify.c index 6b9c27548997..f34f0667ea60 100644 --- a/fs/notify/fanotify/fanotify.c +++ b/fs/notify/fanotify/fanotify.c @@ -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; diff --git a/fs/notify/mark.c b/fs/notify/mark.c index d593d4269561..a10abf90f1bc 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); @@ -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;