Re: [libvirt] [PATCH] lxc: avoid null deref on lxcSetupLoopDevices failure

On 10/27/2011 09:18 AM, ajia(a)redhat.com wrote: >From: Alex Jia&lt;ajia(a)redhat.com&gt; > >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&lt;ajia(a)redhat.com&gt; >--- > 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.