[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] Re: [PATCH] migration: don't segfault on invalid input
From: |
Michael S. Tsirkin |
Subject: |
[Qemu-devel] Re: [PATCH] migration: don't segfault on invalid input |
Date: |
Sun, 17 Oct 2010 23:10:55 +0200 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
On Sun, Oct 17, 2010 at 03:01:45PM -0600, Alex Williamson wrote:
> 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 <address@hidden>
> > ---
> > 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.
Sorry, sent this chunk in error.
> > @@ -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