All of lore.kernel.org
 help / color / mirror / Atom feed
From: Zhong Jinghua <zhongjinghua@huaweicloud.com>
To: josef@toxicpanda.com, axboe@kernel.dk
Cc: linux-block@vger.kernel.org, nbd@other.debian.org,
	linux-kernel@vger.kernel.org, zhongjinghua@huaweicloud.com,
	yi.zhang@huawei.com, yukuai3@huawei.com
Subject: [PATCH -next 3/3] nbd: fix null-ptr-dereference while accessing 'nbd->config'
Date: Fri,  7 Jul 2023 11:15:36 +0800	[thread overview]
Message-ID: <20230707031536.666482-4-zhongjinghua@huaweicloud.com> (raw)
In-Reply-To: <20230707031536.666482-1-zhongjinghua@huaweicloud.com>

From: Zhong Jinghua <zhongjinghua@huawei.com>

nbd->config = config and refcount_set(&nbd->config_refs, 1) in
nbd_genl_connect may be out of order, causing config_refs to be set to 1
first, and then nbd_open accessing nbd->config reports a null pointer
reference.
   T1                      T2
vfs_open
  do_dentry_open
    blkdev_open
      blkdev_get
        __blkdev_get
          nbd_open
           nbd_get_config_unlocked
                        genl_rcv_msg
                          genl_family_rcv_msg
                            genl_family_rcv_msg_doit
                              nbd_genl_connect
                                nbd_alloc_and_init_config
                                  // out of order execution
                                  refcount_set(&nbd->config_refs, 1); // 2
             nbd->config
             // null point
                                  nbd->config = config; // 1

Fix it by adding a cpu memory barrier to guarantee sequential execution.

Signed-off-by: Zhong Jinghua <zhongjinghua@huawei.com>
---
 drivers/block/nbd.c | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
index 7186a9a49445..c410cf29fb0c 100644
--- a/drivers/block/nbd.c
+++ b/drivers/block/nbd.c
@@ -395,8 +395,16 @@ static u32 req_to_nbd_cmd_type(struct request *req)
 
 static struct nbd_config *nbd_get_config_unlocked(struct nbd_device *nbd)
 {
-	if (refcount_inc_not_zero(&nbd->config_refs))
+	if (refcount_inc_not_zero(&nbd->config_refs)) {
+		/*
+		 * Add smp_mb__after_atomic to ensure that reading nbd->config_refs
+		 * and reading nbd->config is ordered. The pair is the barrier in
+		 * nbd_alloc_and_init_config(), avoid nbd->config_refs is set
+		 * before nbd->config.
+		 */
+		smp_mb__after_atomic();
 		return nbd->config;
+	}
 
 	return NULL;
 }
@@ -1555,7 +1563,15 @@ static int nbd_alloc_and_init_config(struct nbd_device *nbd)
 	init_waitqueue_head(&config->conn_wait);
 	config->blksize_bits = NBD_DEF_BLKSIZE_BITS;
 	atomic_set(&config->live_connections, 0);
+
 	nbd->config = config;
+	/*
+	 * Order refcount_set(&nbd->config_refs, 1) and nbd->config assignment,
+	 * its pair is the barrier in nbd_get_config_unlocked().
+	 * So nbd_get_config_unlocked() won't see nbd->config as null after
+	 * refcount_inc_not_zero() succeed.
+	 */
+	smp_mb__before_atomic();
 	refcount_set(&nbd->config_refs, 1);
 
 	return 0;
-- 
2.31.1


  parent reply	other threads:[~2023-07-07  3:18 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-07-07  3:15 [PATCH -next 0/3] nbd: fix null-ptr-dereference while accessing 'nbd->config' Zhong Jinghua
2023-07-07  3:15 ` [PATCH -next 1/3] nbd: fold nbd config initialization into nbd_alloc_config() Zhong Jinghua
2023-09-28  6:05   ` Yu Kuai
2023-07-07  3:15 ` [PATCH -next 2/3] nbd: factor out a helper to get nbd_config without holding 'config_lock' Zhong Jinghua
2023-09-28  6:05   ` Yu Kuai
2023-07-07  3:15 ` Zhong Jinghua [this message]
2023-09-28  6:07   ` [PATCH -next 3/3] nbd: fix null-ptr-dereference while accessing 'nbd->config' Yu Kuai

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230707031536.666482-4-zhongjinghua@huaweicloud.com \
    --to=zhongjinghua@huaweicloud.com \
    --cc=axboe@kernel.dk \
    --cc=josef@toxicpanda.com \
    --cc=linux-block@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=nbd@other.debian.org \
    --cc=yi.zhang@huawei.com \
    --cc=yukuai3@huawei.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.