From ef8c887ee01a4e4c8c5c28c86ea5b45162c51bcd Mon Sep 17 00:00:00 2001 From: Eric Blake Date: Wed, 8 Nov 2017 15:57:03 -0600 Subject: nbd/server: Fix structured read of length 0 The NBD spec was recently clarified to state that a read of length 0 should not be attempted by a compliant client; but that a server must still handle it correctly in an unspecified manner (that is, either a successful no-op or an error reply, but not a crash) [1]. However, it also implies that NBD_REPLY_TYPE_OFFSET_DATA must have a non-zero payload length, but our existing code was replying with a chunk that a picky client could reject as invalid because it was missing a payload (our own client implementation was recently patched to be that picky, after first fixing it to not send 0-length requests). We are already doing successful no-ops for 0-length writes and for non-structured reads; so for consistency, we want structured reply reads to also be a no-op. The easiest way to do this is to return a NBD_REPLY_TYPE_NONE chunk; this is best done via a new helper function (especially since future patches for other structured replies may benefit from using the same helper). [1] https://github.com/NetworkBlockDevice/nbd/commit/ee926037 Signed-off-by: Eric Blake Message-Id: <20171108215703.9295-8-eblake@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy --- nbd/server.c | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) (limited to 'nbd/server.c') diff --git a/nbd/server.c b/nbd/server.c index 6ebb7d9c2e..df771fd42f 100644 --- a/nbd/server.c +++ b/nbd/server.c @@ -1273,6 +1273,21 @@ static inline void set_be_chunk(NBDStructuredReplyChunk *chunk, uint16_t flags, stl_be_p(&chunk->length, length); } +static int coroutine_fn nbd_co_send_structured_done(NBDClient *client, + uint64_t handle, + Error **errp) +{ + NBDStructuredReplyChunk chunk; + struct iovec iov[] = { + {.iov_base = &chunk, .iov_len = sizeof(chunk)}, + }; + + trace_nbd_co_send_structured_done(handle); + set_be_chunk(&chunk, NBD_REPLY_FLAG_DONE, NBD_REPLY_TYPE_NONE, handle, 0); + + return nbd_co_send_iov(client, iov, 1, errp); +} + static int coroutine_fn nbd_co_send_structured_read(NBDClient *client, uint64_t handle, uint64_t offset, @@ -1286,6 +1301,7 @@ static int coroutine_fn nbd_co_send_structured_read(NBDClient *client, {.iov_base = data, .iov_len = size} }; + assert(size); trace_nbd_co_send_structured_read(handle, offset, data, size); set_be_chunk(&chunk.h, NBD_REPLY_FLAG_DONE, NBD_REPLY_TYPE_OFFSET_DATA, handle, sizeof(chunk) - sizeof(chunk.h) + size); @@ -1544,10 +1560,13 @@ reply: if (ret < 0) { ret = nbd_co_send_structured_error(req->client, request.handle, -ret, msg, &local_err); - } else { + } else if (reply_data_len) { ret = nbd_co_send_structured_read(req->client, request.handle, request.from, req->data, reply_data_len, &local_err); + } else { + ret = nbd_co_send_structured_done(req->client, request.handle, + &local_err); } } else { ret = nbd_co_send_simple_reply(req->client, request.handle, -- cgit v1.2.1