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

[Juan Quintela]

"Dr. David Alan Gilbert" <address@hidden> wrote:
> * Juan Quintela (address@hidden) wrote:
>> We still don't put anything there.
>> 
>> Signed-off-by: Juan Quintela <address@hidden>
>> ---
>>  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;
        }
??

Thanks, Juan.