Re: [PATCH v2 for-4.5 1/3] python/xc: Fix multiple issues in pyflask_context_to_sid()

[Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>]

On Tue, Dec 09, 2014 at 04:43:22PM +0000, Andrew Cooper wrote:
> The error handling from a failed memory allocation should return
> PyErr_SetFromErrno(xc_error_obj); rather than simply calling it and continuing
> to the memcpy() below, with the dest pointer being NULL.
> 
> Coverity also complains about passing a non-NUL terminated string to
> xc_flask_context_to_sid().  xc_flask_context_to_sid() doesn't actually take a
> NUL terminated string, but it does take a char* which, in context, used to be
> a string, which is why Coverity complains.
> 
> One solution would be to use strdup(ctx) which is simpler than a
> strlen()/malloc()/memcpy() combo, which would result in a NUL-terminated
> string being used with xc_flask_context_to_sid().
> 
> However, ctx is strictly an input to the hypercall and is not mutated along
> the way.  Both these issues can be fixed, and the error logic simplified, by
> not duplicating ctx in the first place.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Coverity-IDs: 1055305 1055721
> Acked-by: 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>
> 
> ---
> v2: Expand the commit message.  No code change

Thank you.

Release-Acked-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>

> ---
>  tools/python/xen/lowlevel/xc/xc.c |   21 +++------------------
>  1 file changed, 3 insertions(+), 18 deletions(-)
> 
> diff --git a/tools/python/xen/lowlevel/xc/xc.c b/tools/python/xen/lowlevel/xc/xc.c
> index f83e33d..2aa0dc7 100644
> --- a/tools/python/xen/lowlevel/xc/xc.c
> +++ b/tools/python/xen/lowlevel/xc/xc.c
> @@ -2125,8 +2125,6 @@ static PyObject *pyflask_context_to_sid(PyObject *self, PyObject *args,
>  {
>      xc_interface *xc_handle;
>      char *ctx;
> -    char *buf;
> -    uint32_t len;
>      uint32_t sid;
>      int ret;
>  
> @@ -2136,28 +2134,15 @@ static PyObject *pyflask_context_to_sid(PyObject *self, PyObject *args,
>                                        &ctx) )
>          return NULL;
>  
> -    len = strlen(ctx);
> -
> -    buf = malloc(len);
> -    if (!buf) {
> -        errno = -ENOMEM;
> -        PyErr_SetFromErrno(xc_error_obj);
> -    }
> -    
> -    memcpy(buf, ctx, len);
> -    
>      xc_handle = xc_interface_open(0,0,0);
>      if (!xc_handle) {
> -        free(buf);
>          return PyErr_SetFromErrno(xc_error_obj);
>      }
> -    
> -    ret = xc_flask_context_to_sid(xc_handle, buf, len, &sid);
> -        
> +
> +    ret = xc_flask_context_to_sid(xc_handle, ctx, strlen(ctx), &sid);
> +
>      xc_interface_close(xc_handle);
>  
> -    free(buf);
> -    
>      if ( ret != 0 ) {
>          errno = -ret;
>          return PyErr_SetFromErrno(xc_error_obj);
> -- 
> 1.7.10.4
>