There's no need to read into a temporary buffer (oversized since commit 7d3123e1) followed by a byteswap into a uint64_t to check for a magic number via memcmp(), when the code immediately below demonstrates reading into the uint64_t then byteswapping in place and checking for a magic number via integer math. What's more, having a different error message when the server's first reply byte is 0 is unusual - it's no different from any other wrong magic number, and we already detected short reads.
Signed-off-by: Eric Blake <[email protected]> --- nbd/nbd-internal.h | 1 + nbd/client.c | 14 +++----------- 2 files changed, 4 insertions(+), 11 deletions(-) diff --git a/nbd/nbd-internal.h b/nbd/nbd-internal.h index eeff78d3c98..306a533dcd1 100644 --- a/nbd/nbd-internal.h +++ b/nbd/nbd-internal.h @@ -46,6 +46,7 @@ /* Size of oldstyle negotiation */ #define NBD_OLDSTYLE_NEGOTIATE_SIZE (8 + 8 + 8 + 4 + 124) +#define NBD_INIT_MAGIC 0x4e42444d41474943LL #define NBD_REQUEST_MAGIC 0x25609513 #define NBD_OPTS_MAGIC 0x49484156454F5054LL #define NBD_CLIENT_MAGIC 0x0000420281861253LL diff --git a/nbd/client.c b/nbd/client.c index 0be89f9e641..17ee24492a4 100644 --- a/nbd/client.c +++ b/nbd/client.c @@ -731,7 +731,6 @@ int nbd_receive_negotiate(QIOChannel *ioc, const char *name, QIOChannel **outioc, NBDExportInfo *info, Error **errp) { - char buf[256]; uint64_t magic; int rc; bool zeroes = true; @@ -752,21 +751,14 @@ int nbd_receive_negotiate(QIOChannel *ioc, const char *name, goto fail; } - if (nbd_read(ioc, buf, 8, errp) < 0) { + if (nbd_read(ioc, &magic, sizeof(magic), errp) < 0) { error_prepend(errp, "Failed to read data: "); goto fail; } - - buf[8] = '\0'; - if (strlen(buf) == 0) { - error_setg(errp, "Server connection closed unexpectedly"); - goto fail; - } - - magic = ldq_be_p(buf); + magic = be64_to_cpu(magic); trace_nbd_receive_negotiate_magic(magic); - if (memcmp(buf, "NBDMAGIC", 8) != 0) { + if (magic != NBD_INIT_MAGIC) { error_setg(errp, "Invalid magic received"); goto fail; } -- 2.17.2
