Re: [Xen-devel] [PATCH 02/21] libxc: Do not segfault if (e.g.) switch_qemu_logdirty fails

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Fri, 2012-06-15 at 12:53 +0100, Ian Jackson wrote:
> In xc_domain_save the local variable `ob' is initialised to NULL.
> There are then various startup actions.  Some of these `goto out' on
> failure; for example the call to callbacks->switch_qemu_logdirty on
> l.978.  However, out is used both by success and error paths.  So it
> attempts (l.2043) to flush the current output buffer.  If ob has not
> yet been assigned a non-NULL value, this segfaults.  So make the call
> to outbuf_flush conditional on ob.
> 
> Signed-off-by: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>

Doing anything like splitting the error case into a separate path etc
looks too complex to me, so this seems like the best change.

Acked-by: Ian Campbell <ian.campbell@xxxxxxxxxx>

I couldn't help noticing that there are a bunch of "goto out" statements
_after_ the out label. They all appear to be protected by an if (!rc)
and be preceded by an rc = 1, but talk about making it hard for the
reader!

> ---
>  tools/libxc/xc_domain_save.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/tools/libxc/xc_domain_save.c b/tools/libxc/xc_domain_save.c
> index fcc7718..c359649 100644
> --- a/tools/libxc/xc_domain_save.c
> +++ b/tools/libxc/xc_domain_save.c
> @@ -2040,7 +2040,7 @@ int xc_domain_save(xc_interface *xch, int io_fd, 
> uint32_t dom, uint32_t max_iter
>      }
>  
>      /* Flush last write and discard cache for file. */
> -    if ( outbuf_flush(xch, ob, io_fd) < 0 ) {
> +    if ( ob && outbuf_flush(xch, ob, io_fd) < 0 ) {
>          PERROR("Error when flushing output buffer");
>          rc = 1;
>      }



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel