Re: [PATCH] nbd: fix circular lock dependency in nbd_reconnect_socket
"Zhou, Yun" <[email protected]>
| Newsgroups | org.kernel.vger.linux-block,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <[email protected]> |
Friendly ping.
On 6/29/2026 2:03 PM, Yun Zhou wrote:
> Move sk_set_memalloc() out of the tx_lock critical section in
> nbd_reconnect_socket() to break a circular lock dependency.
>
> sk_set_memalloc() internally calls static_branch_inc() which acquires
> cpu_hotplug_lock. When called under tx_lock, this creates the dependency:
>
> tx_lock -> cpu_hotplug_lock
>
> The lockdep splat shows the following circular chain:
>
> cmd->lock -> tx_lock
> from nbd_queue_rq() in the block I/O dispatch path.
>
> tx_lock -> cpu_hotplug_lock
> from nbd_reconnect_socket() calling sk_set_memalloc() under tx_lock.
>
> cpu_hotplug_lock -> fs_reclaim -> q_usage_counter(io)
> from create_worker() during CPU hotplug needing memory allocation,
> which depends on block I/O completion to reclaim memory.
>
> q_usage_counter(io) -> elevator_lock
> from nbd_start_device() -> blk_mq_update_nr_hw_queues() ->
> elevator_change() which freezes the queue then acquires elevator_lock.
>
> elevator_lock -> set->srcu -> cmd->lock
> from elevator_switch() -> blk_mq_quiesce_queue() waiting for srcu,
> which waits for the I/O dispatch path holding cmd->lock.
>
> Fix this by moving sk_set_memalloc() and sk_sndtimeo setup before the
> tx_lock acquisition. This is safe because the new socket has not yet
> been assigned to nsock->sock and is invisible to other code paths. In
> the failure path (no dead connection found), sk_clear_memalloc() is
> called to undo the setup before releasing the socket.
>
> Fixes: b7aa3d39385d ("nbd: add a reconfigure netlink command")
> Reported-by: [email protected]
> Closes: https://syzkaller.appspot.com/bug?extid=3dbc6142c85cc77eaf04
> Signed-off-by: Yun Zhou <[email protected]>
> ---
> drivers/block/nbd.c | 12 +++++++++---
> 1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
> index 497f3bbe5795..c5d3ae8f5fc5 100644
> --- a/drivers/block/nbd.c
> +++ b/drivers/block/nbd.c
> @@ -1392,6 +1392,14 @@ static int nbd_reconnect_socket(struct nbd_device *nbd, unsigned long arg)
> return -ENOMEM;
> }
>
> + /* Setup new socket properties before taking tx_lock to avoid
> + * circular dependency: tx_lock -> cpu_hotplug_lock (via
> + * sk_set_memalloc -> static_branch_inc).
> + */
> + sk_set_memalloc(sock->sk);
> + if (nbd->tag_set.timeout)
> + sock->sk->sk_sndtimeo = nbd->tag_set.timeout;
> +
> for (i = 0; i < config->num_connections; i++) {
> struct nbd_sock *nsock = config->socks[i];
>
> @@ -1403,9 +1411,6 @@ static int nbd_reconnect_socket(struct nbd_device *nbd, unsigned long arg)
> mutex_unlock(&nsock->tx_lock);
> continue;
> }
> - sk_set_memalloc(sock->sk);
> - if (nbd->tag_set.timeout)
> - sock->sk->sk_sndtimeo = nbd->tag_set.timeout;
> atomic_inc(&config->recv_threads);
> refcount_inc(&nbd->config_refs);
> old = nsock->sock;
> @@ -1433,6 +1438,7 @@ static int nbd_reconnect_socket(struct nbd_device *nbd, unsigned long arg)
> wake_up(&config->conn_wait);
> return 0;
> }
> + sk_clear_memalloc(sock->sk);
> sockfd_put(sock);
> kfree(args);
> return -ENOSPC;