[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-block] [PATCH v3 4/6] nbd/client: Support qemu-img convert fro

From: Eric Blake
Subject: Re: [Qemu-block] [PATCH v3 4/6] nbd/client: Support qemu-img convert from unaligned size
Date: Fri, 29 Mar 2019 11:18:31 -0500
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.6.0

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,
>>>                                                   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

Attachment: signature.asc
Description: OpenPGP digital signature

reply via email to

[Prev in Thread] Current Thread [Next in Thread]