diff options
author | Eric Blake <eblake@redhat.com> | 2019-01-17 13:36:42 -0600 |
---|---|---|
committer | Eric Blake <eblake@redhat.com> | 2019-01-21 15:49:51 -0600 |
commit | 7596bbb390838359e4789996f349bda0cad56b0e (patch) | |
tree | c42691a88bb5a3556682937e92fa53326e68bedb | |
parent | 4485936b6de20afa38138e9d1e8ffed88cf0be73 (diff) | |
download | qemu-7596bbb390838359e4789996f349bda0cad56b0e.zip qemu-7596bbb390838359e4789996f349bda0cad56b0e.tar.gz qemu-7596bbb390838359e4789996f349bda0cad56b0e.tar.bz2 |
nbd/server: Hoist length check to qmp_nbd_server_add
We only had two callers to nbd_export_new; qemu-nbd.c always
passed a valid offset/length pair (because it already checked
the file length, to ensure that offset was in bounds), while
blockdev-nbd.c always passed 0/-1. Then nbd_export_new reduces
the size to a multiple of BDRV_SECTOR_SIZE (can only happen
when offset is not sector-aligned, since bdrv_getlength()
currently rounds up) (someday, it would be nice to have
byte-accurate lengths - but not today).
However, I'm finding it easier to work with the code if we are
consistent on having both callers pass in a valid length, and
just assert that things are sane in nbd_export_new, meaning
that no negative values were passed, and that offset+size does
not exceed 63 bits (as that really is a fundamental limit to
later operations, whether we use off_t or uint64_t).
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <20190117193658.16413-6-eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
-rw-r--r-- | blockdev-nbd.c | 10 | ||||
-rw-r--r-- | nbd/server.c | 10 |
2 files changed, 12 insertions, 8 deletions
diff --git a/blockdev-nbd.c b/blockdev-nbd.c index c76d541..d73ac1b 100644 --- a/blockdev-nbd.c +++ b/blockdev-nbd.c @@ -146,6 +146,7 @@ void qmp_nbd_server_add(const char *device, bool has_name, const char *name, BlockDriverState *bs = NULL; BlockBackend *on_eject_blk; NBDExport *exp; + int64_t len; if (!nbd_server) { error_setg(errp, "NBD server not running"); @@ -168,6 +169,13 @@ void qmp_nbd_server_add(const char *device, bool has_name, const char *name, return; } + len = bdrv_getlength(bs); + if (len < 0) { + error_setg_errno(errp, -len, + "Failed to determine the NBD export's length"); + return; + } + if (!has_writable) { writable = false; } @@ -175,7 +183,7 @@ void qmp_nbd_server_add(const char *device, bool has_name, const char *name, writable = false; } - exp = nbd_export_new(bs, 0, -1, name, NULL, bitmap, + exp = nbd_export_new(bs, 0, len, name, NULL, bitmap, writable ? 0 : NBD_FLAG_READ_ONLY, NULL, false, on_eject_blk, errp); if (!exp) { diff --git a/nbd/server.c b/nbd/server.c index 6b13601..51ee809 100644 --- a/nbd/server.c +++ b/nbd/server.c @@ -1495,17 +1495,13 @@ 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; - exp->size = size < 0 ? blk_getlength(blk) : size; - if (exp->size < 0) { - error_setg_errno(errp, -exp->size, - "Failed to determine the NBD export's length"); - goto fail; - } - exp->size -= exp->size % BDRV_SECTOR_SIZE; + assert(size >= 0 && size <= INT64_MAX - dev_offset); + exp->size = QEMU_ALIGN_DOWN(size, BDRV_SECTOR_SIZE); if (bitmap) { BdrvDirtyBitmap *bm = NULL; |