[Devel] [PATCH rh7] block/nbd: Fix NULL pointer dereference

[Konstantin Khorenko]

This particular patch has been dropped, nbd-related big patchset has been applied to 3.10.0-957.27.2.vz7.107.13 instead
which includes following patches:

d740eabf06cf nbd: don't allow invalid blocksize settings
9f9a1fbec525 nbd: stop leaking sockets
8344e50a033b nbd: cleanup workqueue on error properly
fa8c59af2263 nbd: set the logical and physical blocksize properly
b7c7a8b886d4 nbd: cleanup ioctl handling
d699a79d5e6d nbd: use an idr to keep track of nbd devices
614988d2996a nbd: use our own workqueue for recv threads
dd93ee99bb5d nbd: move request validity checking into nbd_send_cmd
f650ce6d71cf nbd: remove REQ_TYPE_DRV_PRIV leftovers
d0ab03a09532 nbd: only set MSG_MORE when we have more to send
023bd33a2e48 nbd: blk_mq_init_queue returns an error code on failure, not NULL
7765e1b74978 nbd: s/<asm/uaccess.h>/<linux/uaccess.h>
b3ac5ea4b143 nbd: use dev_err_ratelimited in io path
56dafc43f17f nbd: reset the setup task for NBD_CLEAR_SOCK
2317acbfe561 nbd: fix 64-bit division
3f7ad2f641a4 nbd: use loff_t for blocksize and nbd_set_size args
d6e807104393 nbd: fix setting of 'error' in NBD_DO_IT ioctl
5e7cdb1e368d nbd: add multi-connection support
f8f957bc9858 nbd: fix use-after-free of rq/bio in the xmit path
1b7e55f6b392 nbd: use BLK_MQ_F_BLOCKING
279b7e0fa44c nbd: allow block mq to deal with timeouts
c5f6eef86780 nbd: use flags instead of bool
f249d493c5a4 nbd: don't shutdown sock with irq's disabled
77ad0aad6ab5 nbd: convert to blkmq
bc5793b7f01e nbd: fix race in ioctl
fd9f6e82e749 nbd: pass the nbd pointer for flags debugfs
d13695af787c drivers: use req op accessor
747bd2b9a411 nbd: use correct div_s64 helper
c6afc9411824 nbd: Create size change events for userspace
d91b8c58af36 nbd: ratelimit error msgs after socket close
2a67d619e134 nbd: Move flag parsing to a function
80bae4aef6bf nbd: Cleanup reset of nbd and bdev after a disconnect
fd74be57a931 nbd: Timeouts are not user requested disconnects
09f95d099813 nbd: Remove signal usage
7520c3eff58a nbd: Fix debugfs error handling
9025442d8a12 nbd: use ->compat_ioctl()
c5dfd122ab01 signal: turn dequeue_signal_lock() into kernel_dequeue_signal()
7b66b17556f3 nbd: Add locking for tasks
6e4fd8c14256 nbd: flags is a u32 variable
68b04cd5e0cc nbd: Rename functions for clearness of recv/send path
15acd5d73b22 nbd: Change 'disconnect' to be boolean
c91e8abb7250 nbd: Add debugfs entries
d0173e130259 nbd: Remove variable 'pid'
aafee3b68860 nbd: Move clear queue debug message
83e6b3679299 nbd: Remove 'harderror' and propagate error properly
3433e7fa4460 nbd: restructure sock_shutdown
4b599bb1c467 nbd: sock_shutdown, remove conditional lock
2ee66facd993 nbd: Fix timeout detection
f62693f3c62c block: have drivers use blk_queue_max_discard_sectors()
5558e621b3e2 nbd: stop using req->cmd
1f29a687da73 nbd: Return error pointer directly
c5bb1ef64813 nbd: Return error code directly
59ca2532729d nbd: Remove fixme that was already fixed
3628e4bb0196 nbd: Restructure debugging prints
3d16c58c86e7 nbd: Fix device bytesize type
05878e5766b0 nbd: Replace kthread_create with kthread_run
82b554d27a50 nbd: Remove kernel internal header
68f31bcbc3bc nbd: fix possible memory leak
962190464214 nbd: zero from and len fields in NBD_CMD_DISCONNECT.
8f0580f8fd68 switch nbd to sockfd_lookup/sockfd_put
9d542e7de534 nbd: remove bogus BUG_ON in NBD_CLEAR_QUE


--
Best regards,

Konstantin Khorenko,
Virtuozzo Linux Kernel Team

On 09/11/2019 03:45 PM, Andrey Ryabinin wrote:
> The following commands trigger NULL-ptr dereference in ioctl(NBD_DO_IT):
>
> 	$ modprobe nbd
> 	$ qemu-img create -f qcow2 xxx 10G
> 	$ while true; do qemu-nbd -v -f qcow2  --detect-zeroes=on xxx -r -c /dev/nbd0 --cache=none --aio=native; done &
> 	$ while true; do qemu-nbd -d /dev/nbd0; done &
>
>  BUG: unable to handle kernel NULL pointer dereference at 0000000000000020
>  IP: [<ffffffffc0a2ae73>] __nbd_ioctl+0x343/0x970 [nbd]
>
>  Call Trace:
>    nbd_ioctl+0x6a/0x1a0 [nbd]
>    blkdev_ioctl+0x2ea/0xa30
>    block_ioctl+0x41/0x50
>    do_vfs_ioctl+0x3b0/0x5a0
>    SyS_ioctl+0xa1/0xc0
>    system_call_fastpath+0x22/0x27
>
> NBD_DO_IT unlocks nbd->tx_lock and accesses nbd->sock in nbd_do_it();
> Parallel ioctl(NBD_CLEAR_SOCK) nullifies nbd->sock which might cause
> NULL-ptr deref in nbd_do_it().
>
> Fix the issue by taking nbd->tx_lock in nbd_do_it() to access nbd->sock.
> This should protect us from parallel NBD_CLEAR_SOCK.
>
> https://jira.sw.ru/browse/PSBM-97690
> Signed-off-by: Andrey Ryabinin <aryabinin at virtuozzo.com>
> ---
>  drivers/block/nbd.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
> index e0c6b623585d..2452b49efd56 100644
> --- a/drivers/block/nbd.c
> +++ b/drivers/block/nbd.c
> @@ -419,7 +419,15 @@ static int nbd_do_it(struct nbd_device *nbd)
>
>  	BUG_ON(nbd->magic != NBD_MAGIC);
>
> +	mutex_lock(&nbd->tx_lock);
> +	if (!nbd->sock) {
> +		mutex_unlock(&nbd->tx_lock);
> +		dev_err(disk_to_dev(nbd->disk), "device_create_file failed!\n");
> +		return -EINVAL;
> +	}
>  	sk_set_memalloc(nbd->sock->sk);
> +	mutex_unlock(&nbd->tx_lock);
> +
>  	nbd->pid = task_pid_nr(current);
>  	ret = device_create_file(disk_to_dev(nbd->disk), &pid_attr);
>  	if (ret) {
>