From patchwork Wed May 11 22:39:44 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Eric Blake X-Patchwork-Id: 621301 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3r4rgg1HRcz9t4F for ; Thu, 12 May 2016 08:41:03 +1000 (AEST) Received: from localhost ([::1]:54490 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1b0coP-0003F9-45 for incoming@patchwork.ozlabs.org; Wed, 11 May 2016 18:41:01 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:55587) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1b0cnR-0001Jo-5k for qemu-devel@nongnu.org; Wed, 11 May 2016 18:40:08 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1b0cnP-0000W2-RJ for qemu-devel@nongnu.org; Wed, 11 May 2016 18:40:01 -0400 Received: from mx1.redhat.com ([209.132.183.28]:44679) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1b0cnM-0000U0-0k; Wed, 11 May 2016 18:39:56 -0400 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 87AED61A0C; Wed, 11 May 2016 22:39:55 +0000 (UTC) Received: from red.redhat.com (ovpn-113-79.phx2.redhat.com [10.3.113.79]) by int-mx11.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u4BMdkAP012079; Wed, 11 May 2016 18:39:55 -0400 From: Eric Blake To: qemu-devel@nongnu.org Date: Wed, 11 May 2016 16:39:44 -0600 Message-Id: <1463006384-7734-12-git-send-email-eblake@redhat.com> In-Reply-To: <1463006384-7734-1-git-send-email-eblake@redhat.com> References: <1463006384-7734-1-git-send-email-eblake@redhat.com> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.24 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.26]); Wed, 11 May 2016 22:39:55 +0000 (UTC) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 209.132.183.28 Subject: [Qemu-devel] [PATCH v4 11/11] nbd: Avoid magic number for NBD max name size X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Kevin Wolf , pbonzini@redhat.com, alex@alex.org.uk, qemu-block@nongnu.org, Max Reitz Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: "Qemu-devel" 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 --- v3: enlarge the limit, and document choice of new value --- 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 3e2d76b..2c753cc 100644 --- a/include/block/nbd.h +++ b/include/block/nbd.h @@ -76,6 +76,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 4659df3..b700100 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 b8de8aa..7ffa168 100644 --- a/nbd/server.c +++ b/nbd/server.c @@ -296,13 +296,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; }