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

Commit Message

Vladimir Sementsov-Ogievskiy April 16, 2021, 8:08 a.m. UTC

From: Roman Kagan <[e-mail]>

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 <[e-mail]>
---
 block/nbd.c | 1 +
 1 file changed, 1 insertion(+)

Comments

Eric Blake May 24, 2021, 9:31 p.m. UTC | #1

On Fri, Apr 16, 2021 at 11:08:39AM +0300, Vladimir Sementsov-Ogievskiy wrote:
> From: Roman Kagan <[e-mail]>
> 
> 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 <[e-mail]>
> ---
>  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 <[e-mail]>

Vladimir Sementsov-Ogievskiy May 25, 2021, 4:47 a.m. UTC | #2

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 <[e-mail]>
>>
>> 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 <[e-mail]>
>> ---
>>   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 <[e-mail]>
>

Patch

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);