python/xc: Fix multiple issues in pyxc_readconsolering()

Don't leak a 16k allocation if PyArg_ParseTupleAndKeywords() or the first
xc_readconsolering() fail.  It is trivial to run throught the processes memory
by repeatedly passing junk parameters to this function.

In the case that the call to xc_readconsolering() in the while loop fails,
reinstate str before breaking out, and passing a spurious pointer to free().

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Coverity-IDs: 1054984 1055906
CC: Ian Campbell <Ian.Campbell@citrix.com>
CC: Ian Jackson <Ian.Jackson@eu.citrix.com>
CC: Wei Liu <wei.liu2@citrix.com>
CC: Xen Coverity Team <coverity@xen.org>
Acked-by: Ian Campbell <ian.campbell@citrix.com>

diff --git a/tools/python/xen/lowlevel/xc/xc.c b/tools/python/xen/lowlevel/xc/xc.c
index d95d4594dfa5effbd902d83bd7261a5780558069..f83e33d05f4ae4cd9c957054997f9c1b62387f1e 100644 (file)
--- a/tools/python/xen/lowlevel/xc/xc.c
+++ b/tools/python/xen/lowlevel/xc/xc.c
@@ -1089,7 +1089,7 @@ static PyObject *pyxc_readconsolering(XcObject *self,
 {
     unsigned int clear = 0, index = 0, incremental = 0;
     unsigned int count = 16384 + 1, size = count;
-    char        *str = malloc(size), *ptr;
+    char        *str, *ptr;
     PyObject    *obj;
     int          ret;
 
@@ -1097,15 +1097,17 @@ static PyObject *pyxc_readconsolering(XcObject *self,
 
     if ( !PyArg_ParseTupleAndKeywords(args, kwds, "|iii", kwd_list,
                                       &clear, &index, &incremental) ||
-         !str )
+         !(str = malloc(size)) )
         return NULL;
 
     ret = xc_readconsolering(self->xc_handle, str, &count, clear,
                              incremental, &index);
-    if ( ret < 0 )
+    if ( ret < 0 ) {
+        free(str);
         return pyxc_error_to_exception(self->xc_handle);
+    }
 
-    while ( !incremental && count == size )
+    while ( !incremental && count == size && ret >= 0 )
     {
         size += count - 1;
         if ( size < count )
@@ -1119,9 +1121,6 @@ static PyObject *pyxc_readconsolering(XcObject *self,
         count = size - count;
         ret = xc_readconsolering(self->xc_handle, str, &count, clear,
                                  1, &index);
-        if ( ret < 0 )
-            break;
-
         count += str - ptr;
         str = ptr;
     }