[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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



On 10/27/2011 10:18 AM, Daniel P. Berrange wrote:
On Thu, Oct 27, 2011 at 10:06:09AM +0200, Peter Krempa wrote:
On 10/27/2011 09:18 AM, ajia redhat com wrote:
From: Alex Jia<ajia redhat com>


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.

No it can't.  If VIR_REALLOC_N fails, it guarentees that the original
pointer is left at its original value. It can't become NULL. This is
critical, because we need to release any FDs that were stored into
loopDevs in earlier iterations of the loop


Uh :/

+    if (loopDevs) {
+        for (i = 0 ; i<   nloopDevs ; i++)
+            VIR_FORCE_CLOSE(loopDevs[i]);


and pushed.

This is actually *wrong*. You now leak any file descriptors that
were stored in loopDevs prior to the VIR_REALLOC_N failure. Please
revert this hunk


Yes, if the pointer is retained, it'll definitely leak the FD's :(.

I'm sorry for not noticing that. :(

Reverted with:

commit 95d3b4de714049e4b6b2033e2db9151ae11d6742
Author: Peter Krempa <pkrempa redhat com>
Date:   Thu Oct 27 10:24:30 2011 +0200

    lxc: Revert zeroing count of allocated items if VIR_REALLOC_N fails

    Previous commit clears number of items alocated in lxcSetupLoopDevices
    if VIR_REALLOC_N fails. In that case, the pointer is not NULL, and
    causes leaking FDs that have been allocated.

     *  src/lxc/lxc_controller.c: revert zeroing array size

diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c
index 7603bc7..024756d 100644
--- a/src/lxc/lxc_controller.c
+++ b/src/lxc/lxc_controller.c
@@ -208,7 +208,6 @@ 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;


Peter




Daniel


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]