On 02/09/2017 09:13 AM, Marc Hartmayer 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); + }
re: your other message - we shouldn't need to check for svc->socks != NULL here (see my response to your self-response). And I *think* we don't need to worry about settinc svc->nsocks = 0 here, since the dispose function can only be called once per object.
Do you see any problem with me removing the "if (svc->socks)" around the for loop and VIR_FREE()? If not, I'll do that and push the result. (ACK, assuming that change is okay with you)
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
