Skip to content

Commit

Permalink
nbd: Limit nbdflags to 16 bits
Browse files Browse the repository at this point in the history 
Rather than asserting that nbdflags is within range, just give
it the correct type to begin with :)  nbdflags corresponds to
the per-export portion of NBD Protocol "transmission flags", which
is 16 bits in response to NBD_OPT_EXPORT_NAME and NBD_OPT_GO.

Furthermore, upstream NBD has never passed the global flags to
the kernel via ioctl(NBD_SET_FLAGS) (the ioctl was first
introduced in NBD 2.9.22; then a latent bug in NBD 3.1 actually
tried to OR the global flags with the transmission flags, with
the disaster that the addition of NBD_FLAG_NO_ZEROES in 3.9
caused all earlier NBD 3.x clients to treat every export as
read-only; NBD 3.10 and later intentionally clip things to 16
bits to pass only transmission flags).  Qemu should follow suit,
since the current two global flags (NBD_FLAG_FIXED_NEWSTYLE
and NBD_FLAG_NO_ZEROES) have no impact on the kernel's behavior
during transmission.

CC: qemu-stable@nongnu.org
Signed-off-by: Eric Blake <eblake@redhat.com>

Message-Id: <1469129688-22848-3-git-send-email-eblake@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
  • Loading branch information
@ebblake @bonzini
ebblake authored and bonzini committed Aug 3, 2016
1 parent 5bee0f4 commit 7423f41
Show file tree
Hide file tree
Showing 5 changed files with 25 additions and 25 deletions.
2 changes: 1 addition & 1 deletion block/nbd-client.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
typedef struct NbdClientSession {
QIOChannelSocket *sioc; /* The master data channel */
QIOChannel *ioc; /* The current I/O channel which may differ (eg TLS) */
uint32_t nbdflags;
uint16_t nbdflags;
off_t size;

CoMutex send_mutex;
Expand Down
6 changes: 3 additions & 3 deletions include/block/nbd.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -90,11 +90,11 @@ ssize_t nbd_wr_syncv(QIOChannel *ioc,
size_t niov,
size_t length,
bool do_read);
int nbd_receive_negotiate(QIOChannel *ioc, const char *name, uint32_t *flags,
int nbd_receive_negotiate(QIOChannel *ioc, const char *name, uint16_t *flags,
QCryptoTLSCreds *tlscreds, const char *hostname,
QIOChannel **outioc,
off_t *size, Error **errp);
int nbd_init(int fd, QIOChannelSocket *sioc, uint32_t flags, off_t size);
int nbd_init(int fd, QIOChannelSocket *sioc, uint16_t flags, off_t size);
ssize_t nbd_send_request(QIOChannel *ioc, struct nbd_request *request);
ssize_t nbd_receive_reply(QIOChannel *ioc, struct nbd_reply *reply);
int nbd_client(int fd);
Expand All @@ -104,7 +104,7 @@ typedef struct NBDExport NBDExport;
typedef struct NBDClient NBDClient;

NBDExport *nbd_export_new(BlockBackend *blk, off_t dev_offset, off_t size,
uint32_t nbdflags, void (*close)(NBDExport *),
uint16_t nbdflags, void (*close)(NBDExport *),
Error **errp);
void nbd_export_close(NBDExport *exp);
void nbd_export_get(NBDExport *exp);
Expand Down
28 changes: 15 additions & 13 deletions nbd/client.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -408,7 +408,7 @@ static QIOChannel *nbd_receive_starttls(QIOChannel *ioc,
}


int nbd_receive_negotiate(QIOChannel *ioc, const char *name, uint32_t *flags,
int nbd_receive_negotiate(QIOChannel *ioc, const char *name, uint16_t *flags,
QCryptoTLSCreds *tlscreds, const char *hostname,
QIOChannel **outioc,
off_t *size, Error **errp)
Expand Down Expand Up @@ -468,7 +468,6 @@ int nbd_receive_negotiate(QIOChannel *ioc, const char *name, uint32_t *flags,
uint32_t opt;
uint32_t namesize;
uint16_t globalflags;
uint16_t exportflags;
bool fixedNewStyle = false;

if (read_sync(ioc, &globalflags, sizeof(globalflags)) !=
Expand All @@ -477,7 +476,6 @@ int nbd_receive_negotiate(QIOChannel *ioc, const char *name, uint32_t *flags,
goto fail;
}
globalflags = be16_to_cpu(globalflags);
*flags = globalflags << 16;
TRACE("Global flags are %" PRIx32, globalflags);
if (globalflags & NBD_FLAG_FIXED_NEWSTYLE) {
fixedNewStyle = true;
Expand Down Expand Up @@ -545,17 +543,15 @@ int nbd_receive_negotiate(QIOChannel *ioc, const char *name, uint32_t *flags,
goto fail;
}
*size = be64_to_cpu(s);
TRACE("Size is %" PRIu64, *size);

if (read_sync(ioc, &exportflags, sizeof(exportflags)) !=
sizeof(exportflags)) {
if (read_sync(ioc, flags, sizeof(*flags)) != sizeof(*flags)) {
error_setg(errp, "Failed to read export flags");
goto fail;
}
exportflags = be16_to_cpu(exportflags);
*flags |= exportflags;
TRACE("Export flags are %" PRIx16, exportflags);
be16_to_cpus(flags);
} else if (magic == NBD_CLIENT_MAGIC) {
uint32_t oldflags;

if (name) {
error_setg(errp, "Server does not support export names");
goto fail;
Expand All @@ -572,16 +568,22 @@ int nbd_receive_negotiate(QIOChannel *ioc, const char *name, uint32_t *flags,
*size = be64_to_cpu(s);
TRACE("Size is %" PRIu64, *size);

if (read_sync(ioc, flags, sizeof(*flags)) != sizeof(*flags)) {
if (read_sync(ioc, &oldflags, sizeof(oldflags)) != sizeof(oldflags)) {
error_setg(errp, "Failed to read export flags");
goto fail;
}
*flags = be32_to_cpu(*flags);
be32_to_cpus(&oldflags);
if (oldflags & ~0xffff) {
error_setg(errp, "Unexpected export flags %0x" PRIx32, oldflags);
goto fail;
}
*flags = oldflags;
} else {
error_setg(errp, "Bad magic received");
goto fail;
}

TRACE("Size is %" PRIu64 ", export flags %" PRIx16, *size, *flags);
if (read_sync(ioc, &buf, 124) != 124) {
error_setg(errp, "Failed to read reserved block");
goto fail;
Expand All @@ -593,7 +595,7 @@ int nbd_receive_negotiate(QIOChannel *ioc, const char *name, uint32_t *flags,
}

#ifdef __linux__
int nbd_init(int fd, QIOChannelSocket *sioc, uint32_t flags, off_t size)
int nbd_init(int fd, QIOChannelSocket *sioc, uint16_t flags, off_t size)
{
unsigned long sectors = size / BDRV_SECTOR_SIZE;
if (size / BDRV_SECTOR_SIZE != sectors) {
Expand Down Expand Up @@ -689,7 +691,7 @@ int nbd_disconnect(int fd)
}

#else
int nbd_init(int fd, QIOChannelSocket *ioc, uint32_t flags, off_t size)
int nbd_init(int fd, QIOChannelSocket *ioc, uint16_t flags, off_t size)
{
return -ENOTSUP;
}
Expand Down
10 changes: 4 additions & 6 deletions nbd/server.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ struct NBDExport {
char *name;
off_t dev_offset;
off_t size;
uint32_t nbdflags;
uint16_t nbdflags;
QTAILQ_HEAD(, NBDClient) clients;
QTAILQ_ENTRY(NBDExport) next;

Expand Down Expand Up @@ -544,8 +544,8 @@ static coroutine_fn int nbd_negotiate(NBDClientNewData *data)
NBDClient *client = data->client;
char buf[8 + 8 + 8 + 128];
int rc;
const int myflags = (NBD_FLAG_HAS_FLAGS | NBD_FLAG_SEND_TRIM |
NBD_FLAG_SEND_FLUSH | NBD_FLAG_SEND_FUA);
const uint16_t myflags = (NBD_FLAG_HAS_FLAGS | NBD_FLAG_SEND_TRIM |
NBD_FLAG_SEND_FLUSH | NBD_FLAG_SEND_FUA);
bool oldStyle;

/* Old style negotiation header without options
Expand Down Expand Up @@ -575,7 +575,6 @@ static coroutine_fn int nbd_negotiate(NBDClientNewData *data)

oldStyle = client->exp != NULL && !client->tlscreds;
if (oldStyle) {
assert ((client->exp->nbdflags & ~65535) == 0);
TRACE("advertising size %" PRIu64 " and flags %x",
client->exp->size, client->exp->nbdflags | myflags);
stq_be_p(buf + 8, NBD_CLIENT_MAGIC);
Expand Down Expand Up @@ -606,7 +605,6 @@ static coroutine_fn int nbd_negotiate(NBDClientNewData *data)
goto fail;
}

assert ((client->exp->nbdflags & ~65535) == 0);
TRACE("advertising size %" PRIu64 " and flags %x",
client->exp->size, client->exp->nbdflags | myflags);
stq_be_p(buf + 18, client->exp->size);
Expand Down Expand Up @@ -810,7 +808,7 @@ static void nbd_eject_notifier(Notifier *n, void *data)
}

NBDExport *nbd_export_new(BlockBackend *blk, off_t dev_offset, off_t size,
uint32_t nbdflags, void (*close)(NBDExport *),
uint16_t nbdflags, void (*close)(NBDExport *),
Error **errp)
{
NBDExport *exp = g_malloc0(sizeof(NBDExport));
Expand Down
4 changes: 2 additions & 2 deletions qemu-nbd.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,7 @@ static void *nbd_client_thread(void *arg)
{
char *device = arg;
off_t size;
uint32_t nbdflags;
uint16_t nbdflags;
QIOChannelSocket *sioc;
int fd;
int ret;
Expand Down Expand Up @@ -465,7 +465,7 @@ int main(int argc, char **argv)
BlockBackend *blk;
BlockDriverState *bs;
off_t dev_offset = 0;
uint32_t nbdflags = 0;
uint16_t nbdflags = 0;
bool disconnect = false;
const char *bindto = "0.0.0.0";
const char *port = NULL;
Expand Down

0 comments on commit 7423f41

Please sign in to comment.