commit 7596bbb390838359e4789996f349bda0cad56b0e
author Eric Blake <eblake@redhat.com> Thu Jan 17 13:36:42 2019 -0600
committer Eric Blake <eblake@redhat.com> Mon Jan 21 15:49:51 2019 -0600
tree c42691a88bb5a3556682937e92fa53326e68bedb
parent 4485936b6de20afa38138e9d1e8ffed88cf0be73

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>

blockdev-nbd.c

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 @@
     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 @@
         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 @@
         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) {

nbd/server.c

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 @@
     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;