Re: [Qemu-devel] [PATCH v12 11/21] migration: Create multifd packet

["Dr. David Alan Gilbert" <dgilbert@redhat.com>]

* Juan Quintela (quintela@redhat.com) wrote:
> "Dr. David Alan Gilbert" <dgilbert@redhat.com> wrote:
> > * Juan Quintela (quintela@redhat.com) wrote:
> >> We still don't put anything there.
> >> 
> >> Signed-off-by: Juan Quintela <quintela@redhat.com>
> >> ---
> >>  migration/ram.c | 137 +++++++++++++++++++++++++++++++++++++++++++++++-
> >>  1 file changed, 136 insertions(+), 1 deletion(-)
> >> +    be32_to_cpus(&packet->magic);
> >> +    if (packet->magic != MULTIFD_MAGIC) {
> >> +        error_setg(errp, "multifd: received packet "
> >> +                   "version %d and expected version %d",
> >> +                   packet->magic, MULTIFD_VERSION);
> >
> > That's mixing magic and version. (Magic's as %x please)
> 
> Oops, fixed.
> 
> 
> >> +    p->seq = be32_to_cpu(packet->seq);
> >> +
> >> +    if (p->pages->used) {
> >> +        block = qemu_ram_block_by_name(packet->ramblock);
> >
> > Do you need to ensure that packet->ramblock is a terminated string
> > first?
> 
> packet->ramblock[255] = 0;
> 
> >
> >> +        if (!block) {
> >> +            error_setg(errp, "multifd: unknown ram block %s",
> >> +                       packet->ramblock);
> >> +            return -1;
> >> +        }
> >> +    }
> >> +
> >> +    for (i = 0; i < p->pages->used; i++) {
> >> +        ram_addr_t offset = be64_to_cpu(packet->offset[i]);
> >> +
> >> +        p->pages->iov[i].iov_base = block->host + offset;
> >
> > I think that needs validating to ensure that the source didn't
> > send us junk and cause us to overwrite after the end of block->host
> 
>         if (offset > block->used_length) {
>             error_setg(errp, "multifd: offest too long %" PRId64
>                        " (max %" PRId64 ")",
>                        offset, block->max_length);
>             return -1;
>         }
> ??

It's probably  (offset + TARGET_PAGE_SIZE) that needs checking
but it needs doing in a wrap-safe way.

Dave

> Thanks, Juan.
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK