Re: [PATCH for-4.5 2/3] python/xc: Fix multiple issues in pyxc_readconsolering()

[Ian Campbell <Ian.Campbell@citrix.com>]

On Thu, 2014-11-27 at 12:34 +0000, Andrew Cooper wrote:
> 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>

> ---
>  tools/python/xen/lowlevel/xc/xc.c |   13 ++++++-------
>  1 file changed, 6 insertions(+), 7 deletions(-)
> 
> diff --git a/tools/python/xen/lowlevel/xc/xc.c b/tools/python/xen/lowlevel/xc/xc.c
> index c70b388..2aa0dc7 100644
> --- 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;
>      }