[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <1a59b683-38e0-82ee-6122-4bd1613a2269@huaweicloud.com>
Date: Thu, 28 Sep 2023 14:07:23 +0800
From: Yu Kuai <yukuai1@...weicloud.com>
To: Zhong Jinghua <zhongjinghua@...weicloud.com>, josef@...icpanda.com,
axboe@...nel.dk
Cc: linux-block@...r.kernel.org, nbd@...er.debian.org,
linux-kernel@...r.kernel.org, yi.zhang@...wei.com,
"yukuai (C)" <yukuai3@...wei.com>
Subject: Re: [PATCH -next 3/3] nbd: fix null-ptr-dereference while accessing
'nbd->config'
在 2023/07/07 11:15, Zhong Jinghua 写道:
> From: Zhong Jinghua <zhongjinghua@...wei.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.
>
LGTM
Reviewed-by: Yu Kuai <yukuai3@...wei.com>
> Signed-off-by: Zhong Jinghua <zhongjinghua@...wei.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;
>
Powered by blists - more mailing lists