[PATCH] nbd: freeze the queue while we're adding connections

[PATCH] nbd: freeze the queue while we're adding connections

[Josef Bacik]

When setting up a device, we can krealloc the config->socks array to add
new sockets to the configuration.  However if we happen to get a IO
request in at this point even though we aren't setup we could hit a UAF,
as we deref config->socks without any locking, assuming that the
configuration was setup already and that ->socks is safe to access it as
we have a reference on the configuration.

But there's nothing really preventing IO from occurring at this point of
the device setup, we don't want to incur the overhead of a lock to
access ->socks when it will never change while the device is running.
To fix this UAF scenario simply freeze the queue if we are adding
sockets.  This will protect us from this particular case without adding
any additional overhead for the normal running case.

Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
 drivers/block/nbd.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
index 6727358e147d..e6ea5d344f87 100644
--- a/drivers/block/nbd.c
+++ b/drivers/block/nbd.c
@@ -1022,6 +1022,12 @@ static int nbd_add_socket(struct nbd_device *nbd, unsigned long arg,
 	if (!sock)
 		return err;
 
+	/*
+	 * We need to make sure we don't get any errant requests while we're
+	 * reallocating the ->socks array.
+	 */
+	blk_mq_freeze_queue(nbd->disk->queue);
+
 	if (!netlink && !nbd->task_setup &&
 	    !test_bit(NBD_RT_BOUND, &config->runtime_flags))
 		nbd->task_setup = current;
@@ -1060,10 +1066,12 @@ static int nbd_add_socket(struct nbd_device *nbd, unsigned long arg,
 	nsock->cookie = 0;
 	socks[config->num_connections++] = nsock;
 	atomic_inc(&config->live_connections);
+	blk_mq_unfreeze_queue(nbd->disk->queue);
 
 	return 0;
 
 put_socket:
+	blk_mq_unfreeze_queue(nbd->disk->queue);
 	sockfd_put(sock);
 	return err;
 }
-- 
2.26.2

Re: [PATCH] nbd: freeze the queue while we're adding connections

[Jens Axboe]

On 1/25/21 10:21 AM, Josef Bacik wrote:
> When setting up a device, we can krealloc the config->socks array to add
> new sockets to the configuration.  However if we happen to get a IO
> request in at this point even though we aren't setup we could hit a UAF,
> as we deref config->socks without any locking, assuming that the
> configuration was setup already and that ->socks is safe to access it as
> we have a reference on the configuration.
> 
> But there's nothing really preventing IO from occurring at this point of
> the device setup, we don't want to incur the overhead of a lock to
> access ->socks when it will never change while the device is running.
> To fix this UAF scenario simply freeze the queue if we are adding
> sockets.  This will protect us from this particular case without adding
> any additional overhead for the normal running case.

Applied, thanks.

-- 
Jens Axboe