nbd: Avoid magic number for NBD max name size
authorEric Blake <eblake@redhat.com>
Wed, 11 May 2016 22:39:44 +0000 (16:39 -0600)
committerPaolo Bonzini <pbonzini@redhat.com>
Thu, 16 Jun 2016 16:39:05 +0000 (18:39 +0200)
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
nbd/client.c
nbd/server.c

index 747bb0aaebd9e0be235656925966ac7e80548e7c..df1f8043386570d8d8878c456c8fae937b500ba2 100644 (file)
@@ -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,
index e8bf9fb54043440475ecf28d52f1b2a5ac392b2b..287487c6c21adb5c4cfe796e671eb108f914ae19 100644 (file)
@@ -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;
         }
index 41067a4bf8b6ab075a5e92e912dee47c589df1b6..a677e266ffac8568781612eefc49aecc7645ce41 100644 (file)
@@ -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;
     }