Because we use NBD_CMD_FLAG_REQ_ONE with NBD_CMD_BLOCK_STATUS, a client in narrow mode should not be able to provoke a server into sending a block status result larger than the client's 32-bit request. But in extended mode, a 64-bit status request must be able to handle a 64-bit status result, once a future patch enables the client requesting extended mode. We can also tolerate a non-compliant server sending the new chunk even when it should not.
Signed-off-by: Eric Blake <ebl...@redhat.com> --- block/nbd.c | 38 +++++++++++++++++++++++++++----------- 1 file changed, 27 insertions(+), 11 deletions(-) diff --git a/block/nbd.c b/block/nbd.c index c5dea864ebb6..bd4a9c407bde 100644 --- a/block/nbd.c +++ b/block/nbd.c @@ -563,13 +563,15 @@ static int nbd_parse_offset_hole_payload(BDRVNBDState *s, */ static int nbd_parse_blockstatus_payload(BDRVNBDState *s, NBDStructuredReplyChunk *chunk, - uint8_t *payload, uint64_t orig_length, - NBDExtent *extent, Error **errp) + uint8_t *payload, bool wide, + uint64_t orig_length, + NBDExtentExt *extent, Error **errp) { uint32_t context_id; + size_t len = wide ? sizeof(*extent) : sizeof(NBDExtent); /* The server succeeded, so it must have sent [at least] one extent */ - if (chunk->length < sizeof(context_id) + sizeof(*extent)) { + if (chunk->length < sizeof(context_id) + len) { error_setg(errp, "Protocol error: invalid payload for " "NBD_REPLY_TYPE_BLOCK_STATUS"); return -EINVAL; @@ -584,8 +586,16 @@ static int nbd_parse_blockstatus_payload(BDRVNBDState *s, return -EINVAL; } - extent->length = payload_advance32(&payload); - extent->flags = payload_advance32(&payload); + if (wide) { + extent->length = payload_advance64(&payload); + extent->flags = payload_advance32(&payload); + if (payload_advance32(&payload) != 0) { + trace_nbd_parse_blockstatus_compliance("non-zero extent padding"); + } + } else { + extent->length = payload_advance32(&payload); + extent->flags = payload_advance32(&payload); + } if (extent->length == 0) { error_setg(errp, "Protocol error: server sent status chunk with " @@ -625,7 +635,7 @@ static int nbd_parse_blockstatus_payload(BDRVNBDState *s, * connection; just ignore trailing extents, and clamp things to * the length of our request. */ - if (chunk->length > sizeof(context_id) + sizeof(*extent)) { + if (chunk->length > sizeof(context_id) + len) { trace_nbd_parse_blockstatus_compliance("more than one extent"); } if (extent->length > orig_length) { @@ -1081,7 +1091,7 @@ static int nbd_co_receive_cmdread_reply(BDRVNBDState *s, uint64_t handle, static int nbd_co_receive_blockstatus_reply(BDRVNBDState *s, uint64_t handle, uint64_t length, - NBDExtent *extent, + NBDExtentExt *extent, int *request_ret, Error **errp) { NBDReplyChunkIter iter; @@ -1098,6 +1108,11 @@ static int nbd_co_receive_blockstatus_reply(BDRVNBDState *s, assert(nbd_reply_is_structured(&reply)); switch (chunk->type) { + case NBD_REPLY_TYPE_BLOCK_STATUS_EXT: + if (!s->info.extended_headers) { + trace_nbd_extended_headers_compliance("block_status_ext"); + } + /* fallthrough */ case NBD_REPLY_TYPE_BLOCK_STATUS: if (received) { nbd_channel_error(s, -EINVAL); @@ -1106,9 +1121,10 @@ static int nbd_co_receive_blockstatus_reply(BDRVNBDState *s, } received = true; - ret = nbd_parse_blockstatus_payload(s, &reply.structured, - payload, length, extent, - &local_err); + ret = nbd_parse_blockstatus_payload( + s, &reply.structured, payload, + chunk->type == NBD_REPLY_TYPE_BLOCK_STATUS_EXT, + length, extent, &local_err); if (ret < 0) { nbd_channel_error(s, ret); nbd_iter_channel_error(&iter, ret, &local_err); @@ -1337,7 +1353,7 @@ static int coroutine_fn nbd_client_co_block_status( int64_t *pnum, int64_t *map, BlockDriverState **file) { int ret, request_ret; - NBDExtent extent = { 0 }; + NBDExtentExt extent = { 0 }; BDRVNBDState *s = (BDRVNBDState *)bs->opaque; Error *local_err = NULL; -- 2.33.1