| Message ID | 20230707031536.666482-4-zhongjinghua@huaweicloud.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | nbd: fix null-ptr-dereference while accessing 'nbd->config' | expand |
在 2023/07/07 11:15, Zhong Jinghua 写道: > 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. > LGTM Reviewed-by: Yu Kuai <yukuai3@huawei.com> > 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; >
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;