On Thu, Feb 09, 2017 at 03:13 PM +0100, Marc Hartmayer <mhartmay@xxxxxxxxxxxxxxxxxx> wrote:
> We have to allocate first and if, and only if, it was successful we
> can set the count. A segfault has occurred in
> virNetServerServiceNewPostExecRestart() when VIR_ALLOC_N(svc->socks,
> n) has failed, but svc->nsocsk = n was already set. Thus
> virObejectUnref(svc) was called and therefore it was possible that
> virNetServerServiceDispose was called => segmentation fault. For
> safeness NULL pointer check were added in
> virNetServerServiceDispose().
>
> Signed-off-by: Marc Hartmayer <mhartmay@xxxxxxxxxxxxxxxxxx>
> Reviewed-by: Boris Fiuczynski <fiuczy@xxxxxxxxxxxxxxxxxx>
> Reviewed-by: Bjoern Walk <bwalk@xxxxxxxxxxxxxxxxxx>
> ---
> src/rpc/virnetserverservice.c | 20 +++++++++++---------
> 1 file changed, 11 insertions(+), 9 deletions(-)
>
> diff --git a/src/rpc/virnetserverservice.c b/src/rpc/virnetserverservice.c
> index 1ef0636..006d041 100644
> --- a/src/rpc/virnetserverservice.c
> +++ b/src/rpc/virnetserverservice.c
> @@ -228,9 +228,9 @@ virNetServerServicePtr virNetServerServiceNewUNIX(const char *path,
> svc->tls = virObjectRef(tls);
> #endif
>
> - svc->nsocks = 1;
> - if (VIR_ALLOC_N(svc->socks, svc->nsocks) < 0)
> + if (VIR_ALLOC_N(svc->socks, 1) < 0)
> goto error;
> + svc->nsocks = 1;
>
> if (virNetSocketNewListenUNIX(path,
> mask,
> @@ -289,9 +289,9 @@ virNetServerServicePtr virNetServerServiceNewFD(int fd,
> svc->tls = virObjectRef(tls);
> #endif
>
> - svc->nsocks = 1;
> - if (VIR_ALLOC_N(svc->socks, svc->nsocks) < 0)
> + if (VIR_ALLOC_N(svc->socks, 1) < 0)
> goto error;
> + svc->nsocks = 1;
>
> if (virNetSocketNewListenFD(fd,
> &svc->socks[0]) < 0)
> @@ -367,9 +367,9 @@ virNetServerServicePtr virNetServerServiceNewPostExecRestart(virJSONValuePtr obj
> goto error;
> }
>
> - svc->nsocks = n;
> - if (VIR_ALLOC_N(svc->socks, svc->nsocks) < 0)
> + if (VIR_ALLOC_N(svc->socks, n) < 0)
> goto error;
> + svc->nsocks = n;
>
> for (i = 0; i < svc->nsocks; i++) {
> virJSONValuePtr child = virJSONValueArrayGet(socks, i);
> @@ -492,9 +492,11 @@ void virNetServerServiceDispose(void *obj)
> virNetServerServicePtr svc = obj;
> size_t i;
>
> - for (i = 0; i < svc->nsocks; i++)
> - virObjectUnref(svc->socks[i]);
> - VIR_FREE(svc->socks);
> + if (svc->socks) {
> + for (i = 0; i < svc->nsocks; i++)
> + virObjectUnref(svc->socks[i]);
> + VIR_FREE(svc->socks);
Should we set svc->nsocks = 0 instead of using a NULL-pointer check (or
do both)?
> + }
>
> #if WITH_GNUTLS
> virObjectUnref(svc->tls);
> --
> 2.5.5
>
--
Beste Grüße / Kind regards
Marc Hartmayer
IBM Deutschland Research & Development GmbH
Vorsitzende des Aufsichtsrats: Martina Koederitz
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
