On 10/27/2011 09:18 AM, ajia@redhat.com wrote:
From: Alex Jia<ajia@redhat.com>
If the function lxcSetupLoopDevices(def,&nloopDevs,&loopDevs) failed, the variable loopDevs will keep a initial NULL value, however, the function VIR_FORCE_CLOSE(loopDevs[i]) will directly deref it.
* rc/lxc/lxc_controller.c: fixed a null pointer dereference.
Signed-off-by: Alex Jia<ajia@redhat.com> --- src/lxc/lxc_controller.c | 7 +++++-- 1 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c index c4e7832..024756d 100644 --- a/src/lxc/lxc_controller.c +++ b/src/lxc/lxc_controller.c @@ -1017,8 +1017,11 @@ cleanup: VIR_FORCE_CLOSE(containerhandshake[0]); VIR_FORCE_CLOSE(containerhandshake[1]);
- for (i = 0 ; i< nloopDevs ; i++) - VIR_FORCE_CLOSE(loopDevs[i]);
Indeed, this situation might happen if memory reallocation fails after some iterations of the loop inside of lxcSetupLoopDevices, leaving nloopDevs assigned to some value, but loopDevs being NULL.
+ if (loopDevs) { + for (i = 0 ; i< nloopDevs ; i++) + VIR_FORCE_CLOSE(loopDevs[i]); + } + VIR_FREE(loopDevs);
if (container> 1) {
ACK. I squashed in a fix for seting the device counter to 0 if this happens. (Well it will be fixed on two places at once, as lxcSetupLoopDevices is called only from here). diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c index 024756d..7603bc7 100644 --- a/src/lxc/lxc_controller.c +++ b/src/lxc/lxc_controller.c @@ -208,6 +208,7 @@ static int lxcSetupLoopDevices(virDomainDefPtr def, size_t *nloopDevs, int **loo VIR_DEBUG("Saving loop fd %d", fd); if (VIR_REALLOC_N(*loopDevs, *nloopDevs+1) < 0) { + *nloopDevs = 0; VIR_FORCE_CLOSE(fd); virReportOOMError(); goto cleanup; and pushed. Peter