[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

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



This reverts commit b98e762e3d71e893b221f871825dc64694cfb258.

Commit b98e762e3d71 ("nbd: freeze the queue while we're adding
connections") added blk_mq_freeze_queue/blk_mq_unfreeze_queue in
nbd_add_socket() to protect krealloc(config->socks) from concurrent I/O
that could cause a Use-After-Free.

However, analysis shows that in all current code paths, concurrent I/O
cannot actually reach nbd_add_socket():

1. nbd_genl_connect() path:
   nbd_add_socket() is called first, and nbd_start_device() -- which
   starts the queue and enables I/O -- is called only after all sockets
   have been added. So the freeze/unfreeze runs against an idle queue,
   marking then waiting on a percpu_ref that is already zero, and then
   resurrecting it -- a pure no-op that burns an RCU grace period per
   socket on multi-core systems.

2. nbd_ioctl(NBD_SET_SOCK) path:
   The task_setup check enforces that only the thread which performed
   the first NBD_SET_SOCK can call NBD_SET_SOCK again. That thread is
   blocked in NBD_DO_IT's wait_event_interruptible, so it cannot issue
   another NBD_SET_SOCK concurrently with I/O. Other threads are
   rejected by the task_setup != current check.

3. nbd_genl_reconfigure() does not call nbd_add_socket() at all; it
   uses nbd_reconnect_socket() which replaces a dead socket in-place
   without reallocating config->socks.

Therefore the freeze/unfreeze provides no actual protection in any
reachable code path, while imposing the cost of blk_mq_freeze_queue
(percpu_ref_kill + RCU grace period wait + percpu_ref_resurrect) on
every socket addition during device setup[1].

Revert the change to eliminate the unnecessary overhead.

Link: https://lore.kernel.org/all/20260327091223.4147956-1-leo.lilong@huaweicloud.com/ [1]
Signed-off-by: Yang Erkun <yangerkun@huawei.com>
---
 drivers/block/nbd.c | 11 +----------
 1 file changed, 1 insertion(+), 10 deletions(-)

diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
index fe63f3c55d0d..9033d996c9a9 100644
--- a/drivers/block/nbd.c
+++ b/drivers/block/nbd.c
@@ -1245,22 +1245,16 @@ static int nbd_add_socket(struct nbd_device *nbd, unsigned long arg,
 	struct socket *sock;
 	struct nbd_sock **socks;
 	struct nbd_sock *nsock;
-	unsigned int memflags;
 	int err;
 
 	/* Arg will be cast to int, check it to avoid overflow */
 	if (arg > INT_MAX)
 		return -EINVAL;
+
 	sock = nbd_get_socket(nbd, arg, &err);
 	if (!sock)
 		return err;
 
-	/*
-	 * We need to make sure we don't get any errant requests while we're
-	 * reallocating the ->socks array.
-	 */
-	memflags = blk_mq_freeze_queue(nbd->disk->queue);
-
 	if (!netlink && !nbd->task_setup &&
 	    !test_bit(NBD_RT_BOUND, &config->runtime_flags))
 		nbd->task_setup = current;
@@ -1300,12 +1294,9 @@ static int nbd_add_socket(struct nbd_device *nbd, unsigned long arg,
 	INIT_WORK(&nsock->work, nbd_pending_cmd_work);
 	socks[config->num_connections++] = nsock;
 	atomic_inc(&config->live_connections);
-	blk_mq_unfreeze_queue(nbd->disk->queue, memflags);
-
 	return 0;
 
 put_socket:
-	blk_mq_unfreeze_queue(nbd->disk->queue, memflags);
 	sockfd_put(sock);
 	return err;
 }
-- 
2.52.0


Reply to: