From: Zhong Jinghua <zhongjinghua@xxxxxxxxxx> Config->socks in sock_shutdown may trigger a UAF problem. The reason is that sock_shutdown does not hold the config_lock, so that nbd_ioctl can release config->socks at this time. T0: NBD_DO_IT T1: NBD_SET_SOCK T0 T1 nbd_ioctl mutex_lock(&nbd->config_lock) // get lock __nbd_ioctl nbd_start_device_ioctl nbd_start_device mutex_unlock(&nbd->config_lock) // relase lock wait_event_interruptible (kill, enter sock_shutdown) sock_shutdown nbd_ioctl mutex_lock(&nbd->config_lock) // get lock __nbd_ioctl nbd_add_socket krealloc kfree(p) //config->socks is NULL nbd_sock *nsock = config->socks // error Fix it by moving config_lock up before sock_shutdown. Link: https://lore.kernel.org/all/ab998dda-80ba-7d8b-0cae-36665826deb5@xxxxxxxxxxxxxxx/ Signed-off-by: Zhong Jinghua <zhongjinghua@xxxxxxxxxx> Signed-off-by: Li Lingfeng <lilingfeng3@xxxxxxxxxx> --- v1->v2: Make comment more detailed. drivers/block/nbd.c | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c index 855fdf5c3b4e..7a044b4726b4 100644 --- a/drivers/block/nbd.c +++ b/drivers/block/nbd.c @@ -92,6 +92,11 @@ struct nbd_config { unsigned long runtime_flags; u64 dead_conn_timeout; + /* + * Anyone who tries to get config->socks needs to be + * protected by config_lock since it may be released + * by krealloc in nbd_add_socket. + */ struct nbd_sock **socks; int num_connections; atomic_t live_connections; @@ -876,6 +881,10 @@ static void recv_work(struct work_struct *work) nbd_mark_nsock_dead(nbd, nsock, 1); mutex_unlock(&nsock->tx_lock); + /* + * recv_work will not get config_lock here if recv_workq is flushed + * in ioctl since nbd_open is holding config_refs. + */ nbd_config_put(nbd); atomic_dec(&config->recv_threads); wake_up(&config->recv_wq); @@ -1417,13 +1426,21 @@ static int nbd_start_device_ioctl(struct nbd_device *nbd) mutex_unlock(&nbd->config_lock); ret = wait_event_interruptible(config->recv_wq, atomic_read(&config->recv_threads) == 0); + + /* + * Get config_lock before sock_shutdown to prevent UAF since nbd_add_socket + * may release config->socks concurrently. + * + * config_lock can be got before flush_workqueue since recv_work will not + * get it in the current scenario. + */ + mutex_lock(&nbd->config_lock); if (ret) { sock_shutdown(nbd); nbd_clear_que(nbd); } flush_workqueue(nbd->recv_workq); - mutex_lock(&nbd->config_lock); nbd_bdev_reset(nbd); /* user requested, ignore socket errors */ if (test_bit(NBD_RT_DISCONNECT_REQUESTED, &config->runtime_flags)) -- 2.39.2
