qemu-block
[Top][All Lists]
Advanced

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

Re: [PATCH] nbd: Prevent NULL pointer dereference in nbd_blockdev_client


From: Alexander Ivanov
Subject: Re: [PATCH] nbd: Prevent NULL pointer dereference in nbd_blockdev_client_closed()
Date: Tue, 18 Jun 2024 10:44:52 +0200
User-agent: Mozilla Thunderbird

Hello Eric,

Do you have any ideas about the bug?

Thank you.

On 6/10/24 14:33, Eric Blake wrote:
On Sat, Jun 08, 2024 at 11:36:59AM GMT, Alexander Ivanov wrote:
There is a bug reproducer in the attachment.
Summarizing the reproducer, you are repeatedly calling QMP
nbd-server-start/nbd-server-stop on qemu as NBD server in one thread,
and repeatedly calling 'qemu-nbd -L' in another, to try and provoke
the race where the server is stopped while qemu-nbd -L is still trying
to grab information.


On 6/7/24 17:00, Alexander Ivanov wrote:
In some cases, the NBD server can be stopped before
nbd_blockdev_client_closed() is called, causing the nbd_server variable
to be nullified. This leads to a NULL pointer dereference when accessing
nbd_server.
Am I correct that the NULL pointer deref was in qemu-nbd in your
reproducer, and not in qemu-kvm?

Add a NULL check for nbd_server to the nbd_blockdev_client_closed()
function to prevent NULL pointer dereference.

Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
---
   blockdev-nbd.c | 3 +++
   1 file changed, 3 insertions(+)

diff --git a/blockdev-nbd.c b/blockdev-nbd.c
index 213012435f..fb1f30ae0d 100644
--- a/blockdev-nbd.c
+++ b/blockdev-nbd.c
@@ -52,6 +52,9 @@ int nbd_server_max_connections(void)
   static void nbd_blockdev_client_closed(NBDClient *client, bool ignored)
   {
       nbd_client_put(client);
+    if (nbd_server == NULL) {
+        return;
+    }
       assert(nbd_server->connections > 0);
While this does indeed avoid the NULL dereference right here, I still
want to understand why nbd_server is getting set to NULL while there
is still an active client, and make sure we don't have any other NULL
derefs.  I'll respond again once I've studied the code a bit more.

       nbd_server->connections--;
       nbd_update_server_watch(nbd_server);
--
Best regards,
Alexander Ivanov

--
Best regards,
Alexander Ivanov




reply via email to

[Prev in Thread] Current Thread [Next in Thread]