| Message ID | 20190910075943.12977-1-vsementsov@virtuozzo.com |
|---|---|
| State | New |
| Headers | show |
| Series | util/qemu-sockets: fix keep_alive handling in inet_connect_saddr | expand |
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
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>
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.
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
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.
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...
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,
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(-)