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(a)linux.vnet.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy(a)linux.vnet.ibm.com>
Reviewed-by: Bjoern Walk <bwalk(a)linux.vnet.ibm.com>
---
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)