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
