qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Paolo Bonzini <pbonzini@redhat.com>
To: qemu-devel@nongnu.org
Subject: [Qemu-devel] [PULL 25/30] nbd: Avoid magic number for NBD max name size
Date: Thu, 16 Jun 2016 16:16:20 +0200	[thread overview]
Message-ID: <1466086585-16526-26-git-send-email-pbonzini@redhat.com> (raw)
In-Reply-To: <1466086585-16526-1-git-send-email-pbonzini@redhat.com>

From: Eric Blake <eblake@redhat.com>

Declare a constant and use that when determining if an export
name fits within the constraints we are willing to support.

Note that upstream NBD recently documented that clients MUST
support export names of 256 bytes (not including trailing NUL),
and SHOULD support names up to 4096 bytes.  4096 is a bit big
(we would lose benefits of stack-allocation of a name array),
and we already have other limits in place (for example, qcow2
snapshot names are clamped around 1024).  So for now, just
stick to the required minimum, as that's easier to audit than
a full-scale support for larger names.

Signed-off-by: Eric Blake <eblake@redhat.com>

Message-Id: <1463006384-7734-12-git-send-email-eblake@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 include/block/nbd.h | 6 ++++++
 nbd/client.c        | 2 +-
 nbd/server.c        | 4 ++--
 3 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/include/block/nbd.h b/include/block/nbd.h
index 747bb0a..df1f804 100644
--- a/include/block/nbd.h
+++ b/include/block/nbd.h
@@ -77,6 +77,12 @@ enum {
 
 /* Maximum size of a single READ/WRITE data buffer */
 #define NBD_MAX_BUFFER_SIZE (32 * 1024 * 1024)
+/* Maximum size of an export name. The NBD spec requires 256 and
+ * suggests that servers support up to 4096, but we stick to only the
+ * required size so that we can stack-allocate the names, and because
+ * going larger would require an audit of more code to make sure we
+ * aren't overflowing some other buffer. */
+#define NBD_MAX_NAME_SIZE 256
 
 ssize_t nbd_wr_syncv(QIOChannel *ioc,
                      struct iovec *iov,
diff --git a/nbd/client.c b/nbd/client.c
index e8bf9fb..287487c 100644
--- a/nbd/client.c
+++ b/nbd/client.c
@@ -210,7 +210,7 @@ static int nbd_receive_list(QIOChannel *ioc, char **name, Error **errp)
             error_setg(errp, "incorrect option name length");
             return -1;
         }
-        if (namelen > 255) {
+        if (namelen > NBD_MAX_NAME_SIZE) {
             error_setg(errp, "export name length too long %" PRIu32, namelen);
             return -1;
         }
diff --git a/nbd/server.c b/nbd/server.c
index a21b536..ba950973 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -286,13 +286,13 @@ static int nbd_negotiate_handle_list(NBDClient *client, uint32_t length)
 static int nbd_negotiate_handle_export_name(NBDClient *client, uint32_t length)
 {
     int rc = -EINVAL;
-    char name[256];
+    char name[NBD_MAX_NAME_SIZE + 1];
 
     /* Client sends:
         [20 ..  xx]   export name (length bytes)
      */
     TRACE("Checking length");
-    if (length > 255) {
+    if (length >= sizeof(name)) {
         LOG("Bad length received");
         goto fail;
     }
-- 
2.5.5

  parent reply	other threads:[~2016-06-16 14:17 UTC|newest]

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-06-16 14:15 [Qemu-devel] [PULL 00/30] KVM, build, NBD, SCSI patches for 2016-06-16 Paolo Bonzini
2016-06-16 14:15 ` [Qemu-devel] [PULL 01/30] configure: Remove unused CONFIG_ZERO_MALLOC setting Paolo Bonzini
2016-06-16 14:15 ` [Qemu-devel] [PULL 02/30] os-posix: include sys/mman.h Paolo Bonzini
2016-06-16 14:15 ` [Qemu-devel] [PULL 03/30] clean-includes: run it once more Paolo Bonzini
2016-06-16 14:15 ` [Qemu-devel] [PULL 04/30] configure: Enable -Werror for MinGW builds, too Paolo Bonzini
2016-06-16 14:16 ` [Qemu-devel] [PULL 05/30] Makefile: Fix tag file generation targets Paolo Bonzini
2016-06-16 14:16 ` [Qemu-devel] [PULL 06/30] Make avx2 configure test work with -O2 Paolo Bonzini
2016-06-16 14:16 ` [Qemu-devel] [PULL 07/30] avx2 configure: Use primitives in test Paolo Bonzini
2016-06-16 14:16 ` [Qemu-devel] [PULL 08/30] configure: Remove unused CONFIG_SIGEV_THREAD_ID switch Paolo Bonzini
2016-06-16 14:16 ` [Qemu-devel] [PULL 09/30] nbd: Don't use *_to_cpup() functions Paolo Bonzini
2016-06-16 14:16 ` [Qemu-devel] [PULL 10/30] nbd: Don't use cpu_to_*w() functions Paolo Bonzini
2016-06-16 14:16 ` [Qemu-devel] [PULL 11/30] nbd: simplify the nbd_request and nbd_reply structs Paolo Bonzini
2016-06-16 14:16 ` [Qemu-devel] [PULL 12/30] target-i386: kvm: cache KVM_GET_SUPPORTED_CPUID data Paolo Bonzini
2016-06-16 14:16 ` [Qemu-devel] [PULL 13/30] scsi-disk: Use (unsigned long) typecasts when using "%lu" format string Paolo Bonzini
2016-06-16 14:16 ` [Qemu-devel] [PULL 14/30] linux-headers: update to Linux 4.6 Paolo Bonzini
2016-06-16 16:09   ` Christian Borntraeger
2016-06-16 16:31     ` Greg Kurz
2016-06-16 14:16 ` [Qemu-devel] [PULL 15/30] KVM: use KVM_CAP_MAX_VCPU_ID Paolo Bonzini
2016-06-16 14:16 ` [Qemu-devel] [PULL 16/30] vl.c: Add '-L help' which lists data dirs Paolo Bonzini
2016-06-16 14:16 ` [Qemu-devel] [PULL 17/30] nbd: Use BDRV_REQ_FUA for better FUA where supported Paolo Bonzini
2016-06-16 14:16 ` [Qemu-devel] [PULL 18/30] nbd: More debug typo fixes, use correct formats Paolo Bonzini
2016-06-16 14:16 ` [Qemu-devel] [PULL 19/30] nbd: Quit server after any write error Paolo Bonzini
2016-06-16 14:16 ` [Qemu-devel] [PULL 20/30] nbd: Improve server handling of bogus commands Paolo Bonzini
2016-06-16 14:16 ` [Qemu-devel] [PULL 21/30] nbd: Reject unknown request flags Paolo Bonzini
2016-06-16 14:16 ` [Qemu-devel] [PULL 22/30] nbd: Group all Linux-specific ioctl code in one place Paolo Bonzini
2016-06-16 14:16 ` [Qemu-devel] [PULL 23/30] nbd: Clean up ioctl handling of qemu-nbd -c Paolo Bonzini
2016-06-16 14:16 ` [Qemu-devel] [PULL 24/30] nbd: Detect servers that send unexpected error values Paolo Bonzini
2016-06-16 14:16 ` Paolo Bonzini [this message]
2016-06-16 14:16 ` [Qemu-devel] [PULL 26/30] scsi: esp: check buffer length before reading scsi command Paolo Bonzini
2016-06-16 14:16 ` [Qemu-devel] [PULL 27/30] scsi: esp: respect FIFO invariant after message phase Paolo Bonzini
2016-06-16 14:16 ` [Qemu-devel] [PULL 28/30] scsi: esp: clean up handle_ti/esp_do_dma if s->do_cmd Paolo Bonzini
2016-06-16 14:16 ` [Qemu-devel] [PULL 29/30] scsi: esp: make cmdbuf big enough for maximum CDB size Paolo Bonzini
2016-06-16 14:16 ` [Qemu-devel] [PULL 30/30] vl: smp_parse: cleanups Paolo Bonzini
2016-06-21 15:40   ` Igor Mammedov
2016-06-21 16:22     ` Andrew Jones
2016-06-16 16:02 ` [Qemu-devel] [PULL 00/30] KVM, build, NBD, SCSI patches for 2016-06-16 Peter Maydell
2016-06-16 16:29   ` Paolo Bonzini
2016-06-16 16:55     ` Peter Maydell
2016-06-16 17:01       ` Paolo Bonzini
2016-06-16 17:08         ` Peter Maydell

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=1466086585-16526-26-git-send-email-pbonzini@redhat.com \
    --to=pbonzini@redhat.com \
    --cc=qemu-devel@nongnu.org \
    /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).