nbd: don't allow reconnect after disconnect

Commit Message

Yu Kuai Jan. 3, 2025, 9:28 a.m. UTC

From: Yu Kuai <[e-mail]>

Following process can cause nbd_config UAF:

1) grab nbd_config temporarily;

2) nbd_genl_disconnect() flush all recv_work() and release the
initial reference:

  nbd_genl_disconnect
   nbd_disconnect_and_put
    nbd_disconnect
     flush_workqueue(nbd->recv_workq)
    if (test_and_clear_bit(NBD_RT_HAS_CONFIG_REF, ...))
     nbd_config_put
     -> due to step 1), reference is still not zero

3) nbd_genl_reconfigure() queue recv_work() again;

  nbd_genl_reconfigure
   config = nbd_get_config_unlocked(nbd)
   if (!config)
   -> succeed
   if (!test_bit(NBD_RT_BOUND, ...))
   -> succeed
   nbd_reconnect_socket
    queue_work(nbd->recv_workq, &args->work)

4) step 1) release the reference;

5) Finially, recv_work() will trigger UAF:

  recv_work
   nbd_config_put(nbd)
   -> nbd_config is freed
   atomic_dec(&config->recv_threads)
   -> UAF

Fix the problem by clearing NBD_RT_BOUND in nbd_genl_disconnect(), so
that nbd_genl_reconfigure() will fail.

Fixes: b7aa3d39385d ("nbd: add a reconfigure netlink command")
Reported-by: [e-mail]
Closes: https://lore.kernel.org/all/[e-mail]/
Signed-off-by: Yu Kuai <[e-mail]>
---
 drivers/block/nbd.c | 1 +
 1 file changed, 1 insertion(+)

Comments

Christoph Hellwig Jan. 6, 2025, 8:36 a.m. UTC | #1

Looks good:

Reviewed-by: Christoph Hellwig <[e-mail]>

Can you wire up the reproduce to blktests?

Yu Kuai Jan. 6, 2025, 11:37 a.m. UTC | #2

Hi,

在 2025/01/06 16:36, Christoph Hellwig 写道:
> Looks good:
> 
> Reviewed-by: Christoph Hellwig <[e-mail]>
> 
Thanks for the review!

> Can you wire up the reproduce to blktests?

However, I don't have reliable reporducer yet, I'll try more. :)

Thanks,
Kuai

> 
> .
>

Jens Axboe Jan. 6, 2025, 2:39 p.m. UTC | #3

On Fri, 03 Jan 2025 17:28:59 +0800, Yu Kuai wrote:
> Following process can cause nbd_config UAF:
> 
> 1) grab nbd_config temporarily;
> 
> 2) nbd_genl_disconnect() flush all recv_work() and release the
> initial reference:
> 
> [...]

Applied, thanks!

[1/1] nbd: don't allow reconnect after disconnect
      commit: 844b8cdc681612ff24df62cdefddeab5772fadf1

Best regards,

Patch

diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
index b1a5af69a66d..259bd57fc529 100644
--- a/drivers/block/nbd.c
+++ b/drivers/block/nbd.c
@@ -2179,6 +2179,7 @@  static void nbd_disconnect_and_put(struct nbd_device *nbd)
 	flush_workqueue(nbd->recv_workq);
 	nbd_clear_que(nbd);
 	nbd->task_setup = NULL;
+	clear_bit(NBD_RT_BOUND, &nbd->config->runtime_flags);
 	mutex_unlock(&nbd->config_lock);
 
 	if (test_and_clear_bit(NBD_RT_HAS_CONFIG_REF,