Re: [PATCH -next 3/3] nbd: fix null-ptr-dereference while accessing 'nbd->config'

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



在 2023/07/07 11:15, Zhong Jinghua 写道:
From: Zhong Jinghua <zhongjinghua@xxxxxxxxxx>

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.

LGTM

Reviewed-by: Yu Kuai <yukuai3@xxxxxxxxxx>

Signed-off-by: Zhong Jinghua <zhongjinghua@xxxxxxxxxx>
---
  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;





[Index of Archives]     [Linux RAID]     [Linux SCSI]     [Linux ATA RAID]     [IDE]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Device Mapper]

  Powered by Linux