Re: [Qemu-devel] [PATCH v3 4/6] nbd/client: Support qemu-img convert from unaligned size

[Eric Blake]

On 3/29/19 10:55 AM, Eric Blake wrote:

>>> @@ -966,7 +985,8 @@ int coroutine_fn 
>>> nbd_client_co_block_status(BlockDriverState *bs,
>>>           .from = offset,
>>>           .len = MIN(MIN_NON_ZERO(QEMU_ALIGN_DOWN(INT_MAX,
>>>                                                   bs->bl.request_alignment),
>>> -                                client->info.max_block), bytes),
>>> +                                client->info.max_block),
>>> +                   MIN(bytes, client->info.size - offset)),
> 
> This is a complex computation...
> 

>>> +    if (offset >= client->info.size) {
>>> +        *pnum = bytes;
>>> +        assert(bytes < BDRV_SECTOR_SIZE);
>>> +        /* Intentionally don't report offset_valid for the hole */
>>> +        return BDRV_BLOCK_ZERO;
>>> +    }
>>> +
>>> +    if (client->info.min_block) {
>>> +        assert(QEMU_IS_ALIGNED(request.len, client->info.min_block));
> 
> ...so the assert is here to say that before we hand a length to the
> server, check that our computation ended up being aligned to the
> server's requirements on input.
> 
>>
>> it will crash if info.size is unaligned..
> 
> It will crash if our client code is buggy, before the server ever gets a
> chance to see things. Our client code should not be buggy (we do not
> want to be sending the server unaligned requests).

Oh, I see what you are saying - if the server gives us a file length of
1023, in spite of also giving us a min_block of 512, we CANNOT access
those trailing bytes - but it can still cause this code to fail the
assertion. What I should do is submit an additional patch on the
handshake code that if the server sends us an inconsistent length and
mandates min_block, then we truncate the size to match what we can
actually access while still staying compliant (at which point,
client->info.size will always be aligned to client->info.min_block in
the rest of the code, such as here, and this assertion is back to being
safe about client-only computations that all the other inputs to the
computation were also aligned).

7/6 coming up soon...

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

signature.asc
Description: OpenPGP digital signature