* Juan Quintela (quint...@redhat.com) wrote:
> "Dr. David Alan Gilbert" <dgilb...@redhat.com> wrote:
> > * Juan Quintela (quint...@redhat.com) wrote:
> >> We still don't put anything there.
> >>
> >> Signed-off-by: Juan Quintela <quint...@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 / dgilb...@redhat.com / Manchester, UK
