[PATCH v3 01/33] block/nbd: fix channel object leak

Vladimir Sementsov-Ogievskiy posted 33 patches 4 years, 9 months ago
There is a newer version of this series
[PATCH v3 01/33] block/nbd: fix channel object leak
Posted by Vladimir Sementsov-Ogievskiy 4 years, 9 months ago
From: Roman Kagan <rvkagan@yandex-team.ru>

nbd_free_connect_thread leaks the channel object if it hasn't been
stolen.

Unref it and fix the leak.

Signed-off-by: Roman Kagan <rvkagan@yandex-team.ru>
---
 block/nbd.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/block/nbd.c b/block/nbd.c
index 1d4668d42d..739ae2941f 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -385,6 +385,7 @@ static void nbd_free_connect_thread(NBDConnectThread *thr)
 {
     if (thr->sioc) {
         qio_channel_close(QIO_CHANNEL(thr->sioc), NULL);
+        object_unref(OBJECT(thr->sioc));
     }
     error_free(thr->err);
     qapi_free_SocketAddress(thr->saddr);
-- 
2.29.2


Re: [PATCH v3 01/33] block/nbd: fix channel object leak
Posted by Eric Blake 4 years, 8 months ago
On Fri, Apr 16, 2021 at 11:08:39AM +0300, Vladimir Sementsov-Ogievskiy wrote:
> From: Roman Kagan <rvkagan@yandex-team.ru>
> 
> nbd_free_connect_thread leaks the channel object if it hasn't been
> stolen.
> 
> Unref it and fix the leak.
> 
> Signed-off-by: Roman Kagan <rvkagan@yandex-team.ru>
> ---
>  block/nbd.c | 1 +
>  1 file changed, 1 insertion(+)

Does nbd_yank() have a similar problem?

At any rate, this makes sense to me.
Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org


Re: [PATCH v3 01/33] block/nbd: fix channel object leak
Posted by Vladimir Sementsov-Ogievskiy 4 years, 8 months ago
25.05.2021 00:31, Eric Blake wrote:
> On Fri, Apr 16, 2021 at 11:08:39AM +0300, Vladimir Sementsov-Ogievskiy wrote:
>> From: Roman Kagan <rvkagan@yandex-team.ru>
>>
>> nbd_free_connect_thread leaks the channel object if it hasn't been
>> stolen.
>>
>> Unref it and fix the leak.
>>
>> Signed-off-by: Roman Kagan <rvkagan@yandex-team.ru>
>> ---
>>   block/nbd.c | 1 +
>>   1 file changed, 1 insertion(+)
> 
> Does nbd_yank() have a similar problem?

No, I think not. nbd_yank() just shutdown the socket to cancel in-flight requests. I doesn't release the state.

> 
> At any rate, this makes sense to me.
> Reviewed-by: Eric Blake <eblake@redhat.com>
> 


-- 
Best regards,
Vladimir