mirrors/qemu
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

nbd: Limit nbdflags to 16 bits

Browse Source 
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>
This commit is contained in:
Eric Blake 2016-07-21 13:34:46 -06:00 committed by Paolo Bonzini
parent 5bee0f4717
commit 7423f41782
5 changed files with 25 additions and 25 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
block/nbd-client.h
View File

@ -20,7 +20,7 @@
typedef struct NbdClientSession { typedef struct NbdClientSession {
QIOChannelSocket *sioc; /* The master data channel */ QIOChannelSocket *sioc; /* The master data channel */
QIOChannel *ioc; /* The current I/O channel which may differ (eg TLS) */ QIOChannel *ioc; /* The current I/O channel which may differ (eg TLS) */
uint32_t nbdflags; uint16_t nbdflags;
off_t size; off_t size;
CoMutex send_mutex; CoMutex send_mutex;

6
include/block/nbd.h
View File

@ -90,11 +90,11 @@ ssize_t nbd_wr_syncv(QIOChannel *ioc,
size_t niov, size_t niov,
size_t length, size_t length,
bool do_read); 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, QCryptoTLSCreds *tlscreds, const char *hostname,
QIOChannel **outioc, QIOChannel **outioc,
off_t *size, Error **errp); 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_send_request(QIOChannel *ioc, struct nbd_request *request);
ssize_t nbd_receive_reply(QIOChannel *ioc, struct nbd_reply *reply); ssize_t nbd_receive_reply(QIOChannel *ioc, struct nbd_reply *reply);
int nbd_client(int fd); int nbd_client(int fd);
@ -104,7 +104,7 @@ typedef struct NBDExport NBDExport;
typedef struct NBDClient NBDClient; typedef struct NBDClient NBDClient;
NBDExport *nbd_export_new(BlockBackend *blk, off_t dev_offset, off_t size, 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); Error **errp);
void nbd_export_close(NBDExport *exp); void nbd_export_close(NBDExport *exp);
void nbd_export_get(NBDExport *exp); void nbd_export_get(NBDExport *exp);

28
nbd/client.c
View File

@ -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, QCryptoTLSCreds *tlscreds, const char *hostname,
QIOChannel **outioc, QIOChannel **outioc,
off_t *size, Error **errp) off_t *size, Error **errp)
@ -468,7 +468,6 @@ int nbd_receive_negotiate(QIOChannel *ioc, const char *name, uint32_t *flags,
uint32_t opt; uint32_t opt;
uint32_t namesize; uint32_t namesize;
uint16_t globalflags; uint16_t globalflags;
uint16_t exportflags;
bool fixedNewStyle = false; bool fixedNewStyle = false;
if (read_sync(ioc, &globalflags, sizeof(globalflags)) != if (read_sync(ioc, &globalflags, sizeof(globalflags)) !=
@ -477,7 +476,6 @@ int nbd_receive_negotiate(QIOChannel *ioc, const char *name, uint32_t *flags,
goto fail; goto fail;
} }
globalflags = be16_to_cpu(globalflags); globalflags = be16_to_cpu(globalflags);
*flags = globalflags << 16;
TRACE("Global flags are %" PRIx32, globalflags); TRACE("Global flags are %" PRIx32, globalflags);
if (globalflags & NBD_FLAG_FIXED_NEWSTYLE) { if (globalflags & NBD_FLAG_FIXED_NEWSTYLE) {
fixedNewStyle = true; fixedNewStyle = true;
@ -545,17 +543,15 @@ int nbd_receive_negotiate(QIOChannel *ioc, const char *name, uint32_t *flags,
goto fail; goto fail;
} }
*size = be64_to_cpu(s); *size = be64_to_cpu(s);
TRACE("Size is %" PRIu64, *size);
if (read_sync(ioc, &exportflags, sizeof(exportflags)) != if (read_sync(ioc, flags, sizeof(*flags)) != sizeof(*flags)) {
sizeof(exportflags)) {
error_setg(errp, "Failed to read export flags"); error_setg(errp, "Failed to read export flags");
goto fail; goto fail;
} }
exportflags = be16_to_cpu(exportflags); be16_to_cpus(flags);
*flags |= exportflags;
TRACE("Export flags are %" PRIx16, exportflags);
} else if (magic == NBD_CLIENT_MAGIC) { } else if (magic == NBD_CLIENT_MAGIC) {
uint32_t oldflags;
if (name) { if (name) {
error_setg(errp, "Server does not support export names"); error_setg(errp, "Server does not support export names");
goto fail; goto fail;
@ -572,16 +568,22 @@ int nbd_receive_negotiate(QIOChannel *ioc, const char *name, uint32_t *flags,
*size = be64_to_cpu(s); *size = be64_to_cpu(s);
TRACE("Size is %" PRIu64, *size); 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"); error_setg(errp, "Failed to read export flags");
goto fail; 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 { } else {
error_setg(errp, "Bad magic received"); error_setg(errp, "Bad magic received");
goto fail; goto fail;
} }
TRACE("Size is %" PRIu64 ", export flags %" PRIx16, *size, *flags);
if (read_sync(ioc, &buf, 124) != 124) { if (read_sync(ioc, &buf, 124) != 124) {
error_setg(errp, "Failed to read reserved block"); error_setg(errp, "Failed to read reserved block");
goto fail; goto fail;
@ -593,7 +595,7 @@ fail:
} }
#ifdef __linux__ #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; unsigned long sectors = size / BDRV_SECTOR_SIZE;
if (size / BDRV_SECTOR_SIZE != sectors) { if (size / BDRV_SECTOR_SIZE != sectors) {
@ -689,7 +691,7 @@ int nbd_disconnect(int fd)
} }
#else #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; return -ENOTSUP;
} }

10
nbd/server.c
View File

@ -63,7 +63,7 @@ struct NBDExport {
char *name; char *name;
off_t dev_offset; off_t dev_offset;
off_t size; off_t size;
uint32_t nbdflags; uint16_t nbdflags;
QTAILQ_HEAD(, NBDClient) clients; QTAILQ_HEAD(, NBDClient) clients;
QTAILQ_ENTRY(NBDExport) next; QTAILQ_ENTRY(NBDExport) next;
@ -544,8 +544,8 @@ static coroutine_fn int nbd_negotiate(NBDClientNewData *data)
NBDClient *client = data->client; NBDClient *client = data->client;
char buf[8 + 8 + 8 + 128]; char buf[8 + 8 + 8 + 128];
int rc; int rc;
const int myflags = (NBD_FLAG_HAS_FLAGS | NBD_FLAG_SEND_TRIM | const uint16_t myflags = (NBD_FLAG_HAS_FLAGS | NBD_FLAG_SEND_TRIM |
NBD_FLAG_SEND_FLUSH | NBD_FLAG_SEND_FUA); NBD_FLAG_SEND_FLUSH | NBD_FLAG_SEND_FUA);
bool oldStyle; bool oldStyle;
/* Old style negotiation header without options /* Old style negotiation header without options
@ -575,7 +575,6 @@ static coroutine_fn int nbd_negotiate(NBDClientNewData *data)
oldStyle = client->exp != NULL && !client->tlscreds; oldStyle = client->exp != NULL && !client->tlscreds;
if (oldStyle) { if (oldStyle) {
assert ((client->exp->nbdflags & ~65535) == 0);
TRACE("advertising size %" PRIu64 " and flags %x", TRACE("advertising size %" PRIu64 " and flags %x",
client->exp->size, client->exp->nbdflags | myflags); client->exp->size, client->exp->nbdflags | myflags);
stq_be_p(buf + 8, NBD_CLIENT_MAGIC); stq_be_p(buf + 8, NBD_CLIENT_MAGIC);
@ -606,7 +605,6 @@ static coroutine_fn int nbd_negotiate(NBDClientNewData *data)
goto fail; goto fail;
} }
assert ((client->exp->nbdflags & ~65535) == 0);
TRACE("advertising size %" PRIu64 " and flags %x", TRACE("advertising size %" PRIu64 " and flags %x",
client->exp->size, client->exp->nbdflags | myflags); client->exp->size, client->exp->nbdflags | myflags);
stq_be_p(buf + 18, client->exp->size); stq_be_p(buf + 18, client->exp->size);
@ -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, 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) Error **errp)
{ {
NBDExport *exp = g_malloc0(sizeof(NBDExport)); NBDExport *exp = g_malloc0(sizeof(NBDExport));

4
qemu-nbd.c
View File

@ -251,7 +251,7 @@ static void *nbd_client_thread(void *arg)
{ {
char *device = arg; char *device = arg;
off_t size; off_t size;
uint32_t nbdflags; uint16_t nbdflags;
QIOChannelSocket *sioc; QIOChannelSocket *sioc;
int fd; int fd;
int ret; int ret;
@ -465,7 +465,7 @@ int main(int argc, char **argv)
BlockBackend *blk; BlockBackend *blk;
BlockDriverState *bs; BlockDriverState *bs;
off_t dev_offset = 0; off_t dev_offset = 0;
uint32_t nbdflags = 0; uint16_t nbdflags = 0;
bool disconnect = false; bool disconnect = false;
const char *bindto = "0.0.0.0"; const char *bindto = "0.0.0.0";
const char *port = NULL; const char *port = NULL;