[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [PATCH v3 04/19] nbd/server: Hoist length check to qemp
From: |
Eric Blake |
Subject: |
Re: [Qemu-block] [PATCH v3 04/19] nbd/server: Hoist length check to qemp_nbd_server_add |
Date: |
Wed, 16 Jan 2019 12:03:49 -0600 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.4.0 |
On 1/15/19 10:58 AM, Eric Blake wrote:
> On 1/15/19 10:26 AM, Vladimir Sementsov-Ogievskiy wrote:
>
>>>> @size is not size of the image, but size of the export, so it may be less
>>>> than dev_offset
>>>> (qemu-nbd.c do "fd_size -= dev_offset" before "nbd_export_new(bs,
>>>> dev_offset, fd_size, "
>>>
>>> But the assert is fine because patch 3/19 fixed qemu-nbd.c to never pass
>>> in dev_offset larger than size (it fails up front if dev_offset is out
>>> of bounds, whether from the -o command line option or from what it read
>>> from the partition header with the -P command line option).
>>>
>>
>> Don't follow =(
>>
>> Assume, image size 3M, and we have offset 2M, i.e. -o 2M.
>>
>> than in qemu-nbd.c, we have
>>
>> fd_size = blk_getlength(blk); # 3M
>> ...
>> fd_size -= dev_offset; # 1M
>> ...
>> export = nbd_export_new(bs, dev_offset, fd_size # bs, 2M, 1M
>>
>> in nbd_export_new:
>>
>> assert(dev_offset <= size); # 2M <= 1M
>>
>> fail.
>
> Ouch, you are right. I don't need the assertion in server.c at all;
> because all callers pass in a validated size, but the validated size has
> no comparable relation to dev_offset.
Here's what I'm considering using instead, merely asserting that the
inputs are non-negative and do not overflow 63 bits:
diff --git i/nbd/server.c w/nbd/server.c
index c9937ccdc2a..3ebcbddaa2c 100644
--- i/nbd/server.c
+++ w/nbd/server.c
@@ -1495,11 +1495,12 @@ NBDExport *nbd_export_new(BlockDriverState *bs,
off_t dev_offset, off_t size,
exp->refcount = 1;
QTAILQ_INIT(&exp->clients);
exp->blk = blk;
+ assert(dev_offset >= 0 && dev_offset <= INT64_MAX);
exp->dev_offset = dev_offset;
exp->name = g_strdup(name);
exp->description = g_strdup(description);
exp->nbdflags = nbdflags;
- assert(dev_offset <= size);
+ assert(size >= 0 && size <= INT64_MAX - dev_offset);
exp->size = QEMU_ALIGN_DOWN(size, BDRV_SECTOR_SIZE);
if (bitmap) {
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
signature.asc
Description: OpenPGP digital signature
- Re: [Qemu-block] [PATCH v3 03/19] qemu-nbd: Sanity check partition bounds, (continued)
Re: [Qemu-block] [Qemu-devel] [PATCH v3 04/19] nbd/server: Hoist length check to qemp_nbd_server_add, Eric Blake, 2019/01/16
[Qemu-block] [PATCH v3 06/19] qemu-nbd: Avoid strtol open-coding, Eric Blake, 2019/01/12
[Qemu-block] [PATCH v3 14/19] nbd/client: Pull out oldstyle size determination, Eric Blake, 2019/01/12
[Qemu-block] [PATCH v3 02/19] qemu-nbd: Enhance man page, Eric Blake, 2019/01/12