1. Patchew
  2. linux
  3. View series

[PATCH] nbd: defer config put in recv_work

Zheng Qixing posted 1 patch 1 month, 1 week ago 
drivers/block/nbd.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
[PATCH] nbd: defer config put in recv_work
Posted by Zheng Qixing 1 month, 1 week ago 
From: Zheng Qixing <zhengqixing@huawei.com>

There is one uaf issue in recv_work when running NBD_CLEAR_SOCK and
NBD_CMD_RECONFIGURE:
  nbd_genl_connect     // conf_ref=2 (connect and recv_work A)
  nbd_open	       // conf_ref=3
  recv_work A done     // conf_ref=2
  NBD_CLEAR_SOCK       // conf_ref=1
  nbd_genl_reconfigure // conf_ref=2 (trigger recv_work B)
  close nbd	       // conf_ref=1
  recv_work B
    config_put         // conf_ref=0
    atomic_dec(&config->recv_threads); -> UAF

Or only running NBD_CLEAR_SOCK:
  nbd_genl_connect   // conf_ref=2
  nbd_open 	     // conf_ref=3
  NBD_CLEAR_SOCK     // conf_ref=2
  close nbd
    nbd_release
      config_put     // conf_ref=1
  recv_work
    config_put 	     // conf_ref=0
    atomic_dec(&config->recv_threads); -> UAF

Commit 87aac3a80af5 ("nbd: call nbd_config_put() before notifying the
waiter") moved nbd_config_put() to run before waking up the waiter in
recv_work, in order to ensure that nbd_start_device_ioctl() would not
be woken up while nbd->task_recv was still uncleared.

However, in nbd_start_device_ioctl(), after being woken up it explicitly
calls flush_workqueue() to make sure all current works are finished.
Therefore, there is no need to move the config put ahead of the wakeup.

Move nbd_config_put() to the end of recv_work, so that the reference is
held for the whole lifetime of the worker thread. This makes sure the
config cannot be freed while recv_work is still running, even if clear
+ reconfigure interleave.

In addition, we don't need to worry about recv_work dropping the last
nbd_put (which causes deadlock):

path A (netlink with NBD_CFLAG_DESTROY_ON_DISCONNECT):
  connect  // nbd_refs=1 (trigger recv_work)
  open nbd // nbd_refs=2
  NBD_CLEAR_SOCK
  close nbd
    nbd_release
      nbd_disconnect_and_put
        flush_workqueue // recv_work done
      nbd_config_put
        nbd_put // nbd_refs=1
      nbd_put // nbd_refs=0
        queue_work

path B (netlink without NBD_CFLAG_DESTROY_ON_DISCONNECT):
  connect  // nbd_refs=2 (trigger recv_work)
  open nbd // nbd_refs=3
  NBD_CLEAR_SOCK // conf_refs=2
  close nbd
    nbd_release
      nbd_config_put // conf_refs=1
      nbd_put // nbd_refs=2
  recv_work done // conf_refs=0, nbd_refs=1
  rmmod // nbd_refs=0

Reported-by: syzbot+56fbf4c7ddf65e95c7cc@syzkaller.appspotmail.com
Closes: https://lore.kernel.org/all/6907edce.a70a0220.37351b.0014.GAE@google.com/T/
Fixes: 87aac3a80af5 ("nbd: make the config put is called before the notifying the waiter")
Depends-on: e2daec488c57 ("nbd: Fix hungtask when nbd_config_put")
Signed-off-by: Zheng Qixing <zhengqixing@huawei.com>
---
 drivers/block/nbd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
index a853c65ac65d..215fc18115b7 100644
--- a/drivers/block/nbd.c
+++ b/drivers/block/nbd.c
@@ -1024,9 +1024,9 @@ static void recv_work(struct work_struct *work)
 	nbd_mark_nsock_dead(nbd, nsock, 1);
 	mutex_unlock(&nsock->tx_lock);
 
-	nbd_config_put(nbd);
 	atomic_dec(&config->recv_threads);
 	wake_up(&config->recv_wq);
+	nbd_config_put(nbd);
 	kfree(args);
 }
 
-- 
2.39.2
Re: [PATCH] nbd: defer config put in recv_work
Posted by Lizhi Xu 1 month, 1 week ago 
On Sat,  8 Nov 2025 15:02:02 +0800, Zheng Qixing wrote:
> Reported-by: syzbot+56fbf4c7ddf65e95c7cc@syzkaller.appspotmail.com
> Closes: https://lore.kernel.org/all/6907edce.a70a0220.37351b.0014.GAE@google.com/T/
> Fixes: 87aac3a80af5 ("nbd: make the config put is called before the notifying the waiter")
> Depends-on: e2daec488c57 ("nbd: Fix hungtask when nbd_config_put")
> Signed-off-by: Zheng Qixing <zhengqixing@huawei.com>
> ---
>  drivers/block/nbd.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
> index a853c65ac65d..215fc18115b7 100644
> --- a/drivers/block/nbd.c
> +++ b/drivers/block/nbd.c
> @@ -1024,9 +1024,9 @@ static void recv_work(struct work_struct *work)
>  	nbd_mark_nsock_dead(nbd, nsock, 1);
>  	mutex_unlock(&nsock->tx_lock);
> 
> -	nbd_config_put(nbd);
>  	atomic_dec(&config->recv_threads);
>  	wake_up(&config->recv_wq);
> +	nbd_config_put(nbd);
>  	kfree(args);
>  }
This only makes the problem more hidden, and that's far from enough.
I tested the same patch on syzbot on October 3rd before you did; you
can check it out here [1].

[1] https://syzkaller.appspot.com/bug?extid=56fbf4c7ddf65e95c7cc
Re: [PATCH] nbd: defer config put in recv_work
Posted by Zheng Qixing 1 month, 1 week ago 
Hi,


在 2025/11/10 8:54, Lizhi Xu 写道:
> On Sat,  8 Nov 2025 15:02:02 +0800, Zheng Qixing wrote:
>> Reported-by: syzbot+56fbf4c7ddf65e95c7cc@syzkaller.appspotmail.com
>> Closes: https://lore.kernel.org/all/6907edce.a70a0220.37351b.0014.GAE@google.com/T/
>> Fixes: 87aac3a80af5 ("nbd: make the config put is called before the notifying the waiter")
>> Depends-on: e2daec488c57 ("nbd: Fix hungtask when nbd_config_put")
>> Signed-off-by: Zheng Qixing <zhengqixing@huawei.com>
>> ---
>>   drivers/block/nbd.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
>> index a853c65ac65d..215fc18115b7 100644
>> --- a/drivers/block/nbd.c
>> +++ b/drivers/block/nbd.c
>> @@ -1024,9 +1024,9 @@ static void recv_work(struct work_struct *work)
>>   	nbd_mark_nsock_dead(nbd, nsock, 1);
>>   	mutex_unlock(&nsock->tx_lock);
>>
>> -	nbd_config_put(nbd);
>>   	atomic_dec(&config->recv_threads);
>>   	wake_up(&config->recv_wq);
>> +	nbd_config_put(nbd);
>>   	kfree(args);
>>   }
> This only makes the problem more hidden, and that's far from enough.
> I tested the same patch on syzbot on October 3rd before you did; you
> can check it out here [1].
>
> [1] https://syzkaller.appspot.com/bug?extid=56fbf4c7ddf65e95c7cc


The same patch was triggered by eslam.medhat1993 on Nov 5 via syzbot,

but it didn't produce the same stack trace. Is this stack trace necessarily

related to the UAF issue in nbd? It seems more likely to be a memory

corruption problem, but I'm not certain.


In addition, this issue arises from the mixed use of netlink and ioctl. 
Since

user-space tools generally do not mix these two interfaces, I believe a 
simple

solution along these lines could effectively avoid the UAF problem.


Regards,

Qixing
Re: [PATCH] nbd: defer config put in recv_work
Posted by Jens Axboe 1 month, 1 week ago 
On Sat, 08 Nov 2025 15:02:02 +0800, Zheng Qixing wrote:
> There is one uaf issue in recv_work when running NBD_CLEAR_SOCK and
> NBD_CMD_RECONFIGURE:
>   nbd_genl_connect     // conf_ref=2 (connect and recv_work A)
>   nbd_open	       // conf_ref=3
>   recv_work A done     // conf_ref=2
>   NBD_CLEAR_SOCK       // conf_ref=1
>   nbd_genl_reconfigure // conf_ref=2 (trigger recv_work B)
>   close nbd	       // conf_ref=1
>   recv_work B
>     config_put         // conf_ref=0
>     atomic_dec(&config->recv_threads); -> UAF
> 
> [...]

Applied, thanks!

[1/1] nbd: defer config put in recv_work
      commit: 9517b82d8d422d426a988b213fdd45c6b417b86d

Best regards,
-- 
Jens Axboe

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.