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

[Peter Krempa]

On 10/27/2011 09:18 AM, ajia at redhat.com wrote:
> From: Alex Jia<ajia at 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 at 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