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
