[Qemu-devel] [PATCH] util/qemu-sockets: fix keep_alive handling in inet_connect_saddr

[Qemu-devel] [PATCH] util/qemu-sockets: fix keep_alive handling in inet_connect_saddr

[Vladimir Sementsov-Ogievskiy]

In "if (saddr->keep_alive) {" we may already be on error path, with
invalid sock < 0. Fix it by returning error earlier.

Reported-by: Coverity (CID 1405300)
Suggested-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 util/qemu-sockets.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
index 98ff3a1cce..bcc06d0e01 100644
--- a/util/qemu-sockets.c
+++ b/util/qemu-sockets.c
@@ -461,12 +461,13 @@ int inet_connect_saddr(InetSocketAddress *saddr, Error **errp)
         }
     }
 
+    freeaddrinfo(res);
+
     if (sock < 0) {
         error_propagate(errp, local_err);
+        return sock;
     }
 
-    freeaddrinfo(res);
-
     if (saddr->keep_alive) {
         int val = 1;
         int ret = qemu_setsockopt(sock, SOL_SOCKET, SO_KEEPALIVE,
-- 
2.18.0

Re: [Qemu-devel] [PATCH] util/qemu-sockets: fix keep_alive handling in inet_connect_saddr

[Daniel P. Berrangé]

On Tue, Sep 10, 2019 at 10:59:43AM +0300, Vladimir Sementsov-Ogievskiy wrote:
> In "if (saddr->keep_alive) {" we may already be on error path, with
> invalid sock < 0. Fix it by returning error earlier.
> 
> Reported-by: Coverity (CID 1405300)
> Suggested-by: Peter Maydell <peter.maydell@linaro.org>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>  util/qemu-sockets.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PATCH] util/qemu-sockets: fix keep_alive handling in inet_connect_saddr

[Stefano Garzarella]

On Tue, Sep 10, 2019 at 10:59:43AM +0300, Vladimir Sementsov-Ogievskiy wrote:
> In "if (saddr->keep_alive) {" we may already be on error path, with
> invalid sock < 0. Fix it by returning error earlier.
> 
> Reported-by: Coverity (CID 1405300)
> Suggested-by: Peter Maydell <peter.maydell@linaro.org>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>  util/qemu-sockets.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)

Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>

Re: [Qemu-devel] [PATCH] util/qemu-sockets: fix keep_alive handling in inet_connect_saddr

[Eric Blake]

[-- Attachment #1.1: Type: text/plain, Size: 827 bytes --]

On 9/10/19 3:03 AM, Daniel P. Berrangé wrote:
> On Tue, Sep 10, 2019 at 10:59:43AM +0300, Vladimir Sementsov-Ogievskiy wrote:
>> In "if (saddr->keep_alive) {" we may already be on error path, with
>> invalid sock < 0. Fix it by returning error earlier.
>>
>> Reported-by: Coverity (CID 1405300)
>> Suggested-by: Peter Maydell <peter.maydell@linaro.org>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>> ---
>>  util/qemu-sockets.c | 5 +++--
>>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>

Thanks.  Will queue through my NBD tree, since that's where the original
problem was introduced.

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [Qemu-devel] [PATCH] util/qemu-sockets: fix keep_alive handling in inet_connect_saddr

[Vladimir Sementsov-Ogievskiy]

10.09.2019 16:59, Eric Blake wrote:
> On 9/10/19 3:03 AM, Daniel P. Berrangé wrote:
>> On Tue, Sep 10, 2019 at 10:59:43AM +0300, Vladimir Sementsov-Ogievskiy wrote:
>>> In "if (saddr->keep_alive) {" we may already be on error path, with
>>> invalid sock < 0. Fix it by returning error earlier.
>>>
>>> Reported-by: Coverity (CID 1405300)
>>> Suggested-by: Peter Maydell <peter.maydell@linaro.org>
>>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>>> ---
>>>   util/qemu-sockets.c | 5 +++--
>>>   1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
> 
> Thanks.  Will queue through my NBD tree, since that's where the original
> problem was introduced.
> 

Please add when queueing:

Fixes: aec21d31756cbd

-- 
Best regards,
Vladimir

Re: [Qemu-devel] [PATCH] util/qemu-sockets: fix keep_alive handling in inet_connect_saddr

[Vladimir Sementsov-Ogievskiy]

11.09.2019 15:08, Vladimir Sementsov-Ogievskiy wrote:
> 10.09.2019 16:59, Eric Blake wrote:
>> On 9/10/19 3:03 AM, Daniel P. Berrangé wrote:
>>> On Tue, Sep 10, 2019 at 10:59:43AM +0300, Vladimir Sementsov-Ogievskiy wrote:
>>>> In "if (saddr->keep_alive) {" we may already be on error path, with
>>>> invalid sock < 0. Fix it by returning error earlier.
>>>>
>>>> Reported-by: Coverity (CID 1405300)
>>>> Suggested-by: Peter Maydell <peter.maydell@linaro.org>
>>>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>>>> ---
>>>>   util/qemu-sockets.c | 5 +++--
>>>>   1 file changed, 3 insertions(+), 2 deletions(-)
>>>
>>> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
>>
>> Thanks.  Will queue through my NBD tree, since that's where the original
>> problem was introduced.
>>
> 
> Please add when queueing:
> 
> Fixes: aec21d31756cbd
> 

Hmm, don't you forget? Don't see it neither in pull request nor in your branches.

-- 
Best regards,
Vladimir

Re: [Qemu-devel] [PATCH] util/qemu-sockets: fix keep_alive handling in inet_connect_saddr

[Eric Blake]

[-- Attachment #1.1: Type: text/plain, Size: 1242 bytes --]

On 9/25/19 6:33 AM, Vladimir Sementsov-Ogievskiy wrote:
> 11.09.2019 15:08, Vladimir Sementsov-Ogievskiy wrote:
>> 10.09.2019 16:59, Eric Blake wrote:
>>> On 9/10/19 3:03 AM, Daniel P. Berrangé wrote:
>>>> On Tue, Sep 10, 2019 at 10:59:43AM +0300, Vladimir Sementsov-Ogievskiy wrote:
>>>>> In "if (saddr->keep_alive) {" we may already be on error path, with
>>>>> invalid sock < 0. Fix it by returning error earlier.
>>>>>
>>>>> Reported-by: Coverity (CID 1405300)
>>>>> Suggested-by: Peter Maydell <peter.maydell@linaro.org>
>>>>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>>>>> ---
>>>>>   util/qemu-sockets.c | 5 +++--
>>>>>   1 file changed, 3 insertions(+), 2 deletions(-)
>>>>
>>>> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
>>>
>>> Thanks.  Will queue through my NBD tree, since that's where the original
>>> problem was introduced.
>>>
>>
>> Please add when queueing:
>>
>> Fixes: aec21d31756cbd
>>
> 
> Hmm, don't you forget? Don't see it neither in pull request nor in your branches.

Thanks for catching this.  Fixing now...

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]