[Qemu-devel] Re: [PATCH] migration: don't segfault on invalid input

[Alex Williamson <alex.williamson@redhat.com>]

On Sun, 2010-10-17 at 20:43 +0200, Michael S. Tsirkin wrote:
> host_from_stream_offset returns NULL on error,
> return error instead of trying to use that address,
> to avoid segfault on invalid stream.
> 
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
>  arch_init.c |    5 +++++
>  1 files changed, 5 insertions(+), 0 deletions(-)
> 
> diff --git a/arch_init.c b/arch_init.c
> index e468c0c..bc7528d 100644
> --- a/arch_init.c
> +++ b/arch_init.c
> @@ -116,6 +116,8 @@ static int ram_save_block(QEMUFile *f)
>  
>      if (!block)
>          block = QLIST_FIRST(&ram_list.blocks);
> +    if (!last_block)
> +	last_block = block;
>  
>      current_addr = block->offset + offset;

NAK, last_block == block will cause us to set the continue flag on the
first block.  I assume you're trying to prevent the last_block->offset
segv in the while test, but that should never happen.  The only time
last_block is NULL is at the beginning of stage 1, where all pages are
dirty.  At that point we should always enter the if {} block at the
beginning of the do {} while, which breaks out rather than hitting the
segv.  We'll then set last_block for the next pass.

> @@ -390,6 +392,9 @@ int ram_load(QEMUFile *f, void *opaque, int version_id)
>                  host = qemu_get_ram_ptr(addr);
>              else
>                  host = host_from_stream_offset(f, addr, flags);
> +            if (!host) {
> +                return -EINVAL;
> +            }
>  

This should also never happen since we've synchronized ramblocks at the
beginning of migration, but probably a good idea to return an error.
Thanks,

Alex