On 23 Apr 2016, at 00:40, Eric Blake <ebl...@redhat.com> 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 <ebl...@redhat.com> Reviewed-by: Alex Bligh <a...@alex.org.uk> > > --- > 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