[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [Qemu-devel] [PATCH v3 29/44] nbd: Avoid magic number f
From: |
Alex Bligh |
Subject: |
Re: [Qemu-block] [Qemu-devel] [PATCH v3 29/44] nbd: Avoid magic number for NBD max name size |
Date: |
Mon, 25 Apr 2016 10:32:29 +0100 |
On 23 Apr 2016, at 00:40, Eric Blake <address@hidden> wrote:
> 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.
>
> Signed-off-by: Eric Blake <address@hidden>
Reviewed-by: Alex Bligh <address@hidden>
>
> ---
> 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 fa05a73..a20bf8a 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;
> }
> --
> 2.5.5
>
>
--
Alex Bligh
- [Qemu-block] [PATCH v3 28/44] nbd: Detect servers that send unexpected error values, (continued)
- [Qemu-block] [PATCH v3 28/44] nbd: Detect servers that send unexpected error values, Eric Blake, 2016/04/22
- [Qemu-block] [PATCH v3 27/44] nbd: Use BDRV_REQ_FUA for better FUA where supported, Eric Blake, 2016/04/22
- [Qemu-block] [PATCH v3 26/44] qemu-io: Add 'write -z -u' to test MAY_UNMAP flag, Eric Blake, 2016/04/22
- [Qemu-block] [PATCH v3 31/44] nbd: Share common reply-sending code in server, Eric Blake, 2016/04/22
- [Qemu-block] [PATCH v3 33/44] nbd: Let client skip portions of server reply, Eric Blake, 2016/04/22
- [Qemu-block] [PATCH v3 35/44] nbd: Support shorter handshake, Eric Blake, 2016/04/22
- [Qemu-block] [PATCH v3 30/44] nbd: Treat flags vs. command type as separate fields, Eric Blake, 2016/04/22
- [Qemu-block] [PATCH v3 29/44] nbd: Avoid magic number for NBD max name size, Eric Blake, 2016/04/22
- Re: [Qemu-block] [Qemu-devel] [PATCH v3 29/44] nbd: Avoid magic number for NBD max name size,
Alex Bligh <=
- [Qemu-block] [PATCH v3 38/44] block: Add blk_get_opt_transfer_length(), Eric Blake, 2016/04/22
- [Qemu-block] [PATCH v3 41/44] nbd: Implement NBD_CMD_WRITE_ZEROES on server, Eric Blake, 2016/04/22
- [Qemu-block] [PATCH v3 43/44] nbd: Implement NBD_OPT_BLOCK_SIZE on server, Eric Blake, 2016/04/22
- [Qemu-block] [PATCH v3 44/44] nbd: Implement NBD_OPT_BLOCK_SIZE on client, Eric Blake, 2016/04/22
- [Qemu-block] [PATCH v3 34/44] nbd: Less allocation during NBD_OPT_LIST, Eric Blake, 2016/04/22