qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Eric Blake <eblake@redhat.com>
To: qemu-devel@nongnu.org
Cc: nsoffer@redhat.com, rjones@redhat.com, jsnow@redhat.com,
	vsementsov@virtuozzo.com, qemu-block@nongnu.org,
	Kevin Wolf <kwolf@redhat.com>, Max Reitz <mreitz@redhat.com>
Subject: [Qemu-devel] [PATCH v4 06/21] nbd/server: Favor [u]int64_t over off_t
Date: Thu, 17 Jan 2019 13:36:43 -0600	[thread overview]
Message-ID: <20190117193658.16413-7-eblake@redhat.com> (raw)
In-Reply-To: <20190117193658.16413-1-eblake@redhat.com>

Although our compile-time environment is set up so that we always
support long files with 64-bit off_t, we have no guarantee whether
off_t is the same type as int64_t.  This requires casts when
printing values, and prevents us from directly using qemu_strtoi64()
(which will be done in the next patch). Let's just flip to uint64_t
where possible, and stick to int64_t for detecting failure of
blk_getlength(); we also keep the assertions added in the previous
patch that the resulting values fit in 63 bits.  The overflow check
in nbd_co_receive_request() was already sane (request->from is
validated to fit in 63 bits, and request->len is 32 bits, so the
addition can't overflow 64 bits), but rewrite it in a form easier
to recognize as a typical overflow check.

Rename the variable 'description' to keep line lengths reasonable.

Suggested-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Signed-off-by: Eric Blake <eblake@redhat.com>

---
v4: a few more tweaks
v3: new patch
---
 include/block/nbd.h |  4 ++--
 nbd/server.c        | 18 +++++++++---------
 qemu-nbd.c          | 29 +++++++++++------------------
 3 files changed, 22 insertions(+), 29 deletions(-)

diff --git a/include/block/nbd.h b/include/block/nbd.h
index 1971b557896..24be9570bb4 100644
--- a/include/block/nbd.h
+++ b/include/block/nbd.h
@@ -294,8 +294,8 @@ int nbd_errno_to_system_errno(int err);
 typedef struct NBDExport NBDExport;
 typedef struct NBDClient NBDClient;

-NBDExport *nbd_export_new(BlockDriverState *bs, off_t dev_offset, off_t size,
-                          const char *name, const char *description,
+NBDExport *nbd_export_new(BlockDriverState *bs, uint64_t dev_offset,
+                          uint64_t size, const char *name, const char *desc,
                           const char *bitmap, uint16_t nbdflags,
                           void (*close)(NBDExport *), bool writethrough,
                           BlockBackend *on_eject_blk, Error **errp);
diff --git a/nbd/server.c b/nbd/server.c
index 51ee8094e02..cb0d5634fa1 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -77,8 +77,8 @@ struct NBDExport {
     BlockBackend *blk;
     char *name;
     char *description;
-    off_t dev_offset;
-    off_t size;
+    uint64_t dev_offset;
+    uint64_t size;
     uint16_t nbdflags;
     QTAILQ_HEAD(, NBDClient) clients;
     QTAILQ_ENTRY(NBDExport) next;
@@ -1455,8 +1455,8 @@ static void nbd_eject_notifier(Notifier *n, void *data)
     nbd_export_close(exp);
 }

-NBDExport *nbd_export_new(BlockDriverState *bs, off_t dev_offset, off_t size,
-                          const char *name, const char *description,
+NBDExport *nbd_export_new(BlockDriverState *bs, uint64_t dev_offset,
+                          uint64_t size, const char *name, const char *desc,
                           const char *bitmap, uint16_t nbdflags,
                           void (*close)(NBDExport *), bool writethrough,
                           BlockBackend *on_eject_blk, Error **errp)
@@ -1495,12 +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);
+    assert(dev_offset <= INT64_MAX);
     exp->dev_offset = dev_offset;
     exp->name = g_strdup(name);
-    exp->description = g_strdup(description);
+    exp->description = g_strdup(desc);
     exp->nbdflags = nbdflags;
-    assert(size >= 0 && size <= INT64_MAX - dev_offset);
+    assert(size <= INT64_MAX - dev_offset);
     exp->size = QEMU_ALIGN_DOWN(size, BDRV_SECTOR_SIZE);

     if (bitmap) {
@@ -2130,10 +2130,10 @@ static int nbd_co_receive_request(NBDRequestData *req, NBDRequest *request,
         return -EROFS;
     }
     if (request->from > client->exp->size ||
-        request->from + request->len > client->exp->size) {
+        request->len > client->exp->size - request->from) {
         error_setg(errp, "operation past EOF; From: %" PRIu64 ", Len: %" PRIu32
                    ", Size: %" PRIu64, request->from, request->len,
-                   (uint64_t)client->exp->size);
+                   client->exp->size);
         return (request->type == NBD_CMD_WRITE ||
                 request->type == NBD_CMD_WRITE_ZEROES) ? -ENOSPC : -EINVAL;
     }
diff --git a/qemu-nbd.c b/qemu-nbd.c
index 5c90c5e55f7..598caa656b2 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -176,7 +176,7 @@ static void read_partition(uint8_t *p, struct partition_record *r)
 }

 static int find_partition(BlockBackend *blk, int partition,
-                          off_t *offset, off_t *size)
+                          uint64_t *offset, uint64_t *size)
 {
     struct partition_record mbr[4];
     uint8_t data[MBR_SIZE];
@@ -500,14 +500,14 @@ int main(int argc, char **argv)
 {
     BlockBackend *blk;
     BlockDriverState *bs;
-    off_t dev_offset = 0;
+    uint64_t dev_offset = 0;
     uint16_t nbdflags = 0;
     bool disconnect = false;
     const char *bindto = NULL;
     const char *port = NULL;
     char *sockpath = NULL;
     char *device = NULL;
-    off_t fd_size;
+    int64_t fd_size;
     QemuOpts *sn_opts = NULL;
     const char *sn_id_or_name = NULL;
     const char *sopt = "hVb:o:p:rsnP:c:dvk:e:f:tl:x:T:D:B:";
@@ -665,10 +665,6 @@ int main(int argc, char **argv)
                 error_report("Invalid offset `%s'", optarg);
                 exit(EXIT_FAILURE);
             }
-            if (dev_offset < 0) {
-                error_report("Offset must be positive `%s'", optarg);
-                exit(EXIT_FAILURE);
-            }
             break;
         case 'l':
             if (strstart(optarg, SNAPSHOT_OPT_BASE, NULL)) {
@@ -1005,15 +1001,14 @@ int main(int argc, char **argv)
     }

     if (dev_offset >= fd_size) {
-        error_report("Offset (%lld) has to be smaller than the image size "
-                     "(%lld)",
-                     (long long int)dev_offset, (long long int)fd_size);
+        error_report("Offset (%" PRIu64 ") has to be smaller than the image "
+                     "size (%" PRId64 ")", dev_offset, fd_size);
         exit(EXIT_FAILURE);
     }
     fd_size -= dev_offset;

     if (partition != -1) {
-        off_t limit;
+        uint64_t limit;

         if (dev_offset) {
             error_report("Cannot request partition and offset together");
@@ -1027,15 +1022,13 @@ int main(int argc, char **argv)
         }
         /*
          * MBR partition limits are (32-bit << 9); this assert lets
-         * the compiler know that we have two positive values that
-         * can't overflow 64 bits.
+         * the compiler know that we can't overflow 64 bits.
          */
-        assert(dev_offset >= 0 && dev_offset + limit >= dev_offset);
+        assert(dev_offset + limit >= dev_offset);
         if (dev_offset + limit > fd_size) {
-            error_report("Discovered partition %d at offset %lld size %lld, "
-                         "but size exceeds file length %lld", partition,
-                         (long long int) dev_offset, (long long int) limit,
-                         (long long int) fd_size);
+            error_report("Discovered partition %d at offset %" PRIu64
+                         " size %" PRIu64 ", but size exceeds file length %"
+                         PRId64, partition, dev_offset, limit, fd_size);
             exit(EXIT_FAILURE);
         }
         fd_size = limit;
-- 
2.20.1

  parent reply	other threads:[~2019-01-17 19:37 UTC|newest]

Thread overview: 41+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-01-17 19:36 [Qemu-devel] [PATCH v4 00/21] nbd: add qemu-nbd --list Eric Blake
2019-01-17 19:36 ` [Qemu-devel] [PATCH v4 01/21] iotests: Make 233 output more reliable Eric Blake
2019-01-18  8:28   ` Vladimir Sementsov-Ogievskiy
2019-01-18 10:04     ` Daniel P. Berrangé
2019-01-18 10:02   ` Daniel P. Berrangé
2019-01-18 15:56     ` Eric Blake
2019-01-17 19:36 ` [Qemu-devel] [PATCH v4 02/21] maint: Allow for EXAMPLES in texi2pod Eric Blake
2019-01-18 12:41   ` Vladimir Sementsov-Ogievskiy
2019-01-17 19:36 ` [Qemu-devel] [PATCH v4 03/21] qemu-nbd: Enhance man page Eric Blake
2019-01-17 19:36 ` [Qemu-devel] [PATCH v4 04/21] qemu-nbd: Sanity check partition bounds Eric Blake
2019-01-17 19:36 ` [Qemu-devel] [PATCH v4 05/21] nbd/server: Hoist length check to qmp_nbd_server_add Eric Blake
2019-01-18  9:48   ` Vladimir Sementsov-Ogievskiy
2019-01-17 19:36 ` Eric Blake [this message]
2019-01-17 19:36 ` [Qemu-devel] [PATCH v4 07/21] qemu-nbd: Avoid strtol open-coding Eric Blake
2019-01-17 19:36 ` [Qemu-devel] [PATCH v4 08/21] nbd/client: Refactor nbd_receive_list() Eric Blake
2019-01-17 19:36 ` [Qemu-devel] [PATCH v4 09/21] nbd/client: Move export name into NBDExportInfo Eric Blake
2019-01-17 19:36 ` [Qemu-devel] [PATCH v4 10/21] nbd/client: Change signature of nbd_negotiate_simple_meta_context() Eric Blake
2019-01-17 19:36 ` [Qemu-devel] [PATCH v4 11/21] nbd/client: Split out nbd_send_meta_query() Eric Blake
2019-01-18  9:59   ` Vladimir Sementsov-Ogievskiy
2019-01-17 19:36 ` [Qemu-devel] [PATCH v4 12/21] nbd/client: Split out nbd_receive_one_meta_context() Eric Blake
2019-01-17 19:36 ` [Qemu-devel] [PATCH v4 13/21] nbd/client: Refactor return of nbd_receive_negotiate() Eric Blake
2019-01-17 19:36 ` [Qemu-devel] [PATCH v4 14/21] nbd/client: Split handshake into two functions Eric Blake
2019-01-17 19:36 ` [Qemu-devel] [PATCH v4 15/21] nbd/client: Pull out oldstyle size determination Eric Blake
2019-01-18 10:04   ` Vladimir Sementsov-Ogievskiy
2019-01-17 19:36 ` [Qemu-devel] [PATCH v4 16/21] nbd/client: Refactor nbd_opt_go() to support NBD_OPT_INFO Eric Blake
2019-01-18 11:54   ` Vladimir Sementsov-Ogievskiy
2019-01-17 19:36 ` [Qemu-devel] [PATCH v4 17/21] nbd/client: Add nbd_receive_export_list() Eric Blake
2019-01-18 12:07   ` Vladimir Sementsov-Ogievskiy
2019-01-17 19:36 ` [Qemu-devel] [PATCH v4 18/21] nbd/client: Add meta contexts to nbd_receive_export_list() Eric Blake
2019-01-17 19:36 ` [Qemu-devel] [PATCH v4 19/21] qemu-nbd: Add --list option Eric Blake
2019-01-18 12:28   ` Vladimir Sementsov-Ogievskiy
2019-01-17 19:36 ` [Qemu-devel] [PATCH v4 20/21] nbd/client: Work around 3.0 bug for listing meta contexts Eric Blake
2019-01-17 19:36 ` [Qemu-devel] [PATCH v4 21/21] iotests: Enhance 223, 233 to cover 'qemu-nbd --list' Eric Blake
2019-01-18  8:15 ` [Qemu-devel] [PATCH v4 00/21] nbd: add qemu-nbd --list Vladimir Sementsov-Ogievskiy
2019-01-18 13:47   ` Vladimir Sementsov-Ogievskiy
2019-01-18 16:06     ` Eric Blake
2019-01-18 22:47     ` Eric Blake
2019-01-19  8:07       ` Richard W.M. Jones
2019-01-19 11:39       ` Richard W.M. Jones
2019-01-20 17:42         ` Richard W.M. Jones
2019-01-18 16:08 ` Eric Blake

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190117193658.16413-7-eblake@redhat.com \
    --to=eblake@redhat.com \
    --cc=jsnow@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=mreitz@redhat.com \
    --cc=nsoffer@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=rjones@redhat.com \
    --cc=vsementsov@virtuozzo.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).