Re: [Libguestfs] [libnbd PATCH 3/8] pread: Reject server SR read response with no data chunks

The NBD spec requires that a server doing structured reads must not succeed unless it covers the entire buffer with reply chunks. In the general case, this requires a lot of bookkeeping to check whether offsets were non-overlapping and sufficient, and we'd rather defer such checking to an optional callback function. But a callback function will only be reached per chunk, while we still want to fail the overall read if the callback function was never called because the server erroneously replied with NBD_REPLY_TYPE_NONE with no other chunks instead of an expected NBD_REPLY_TYPE_ERROR*. For this specific error case, the bookkeeping is much simpler - we merely track if we've seen at least one data chunk.

generator/states-reply-simple.c | 1 + generator/states-reply-structured.c | 2 ++ lib/aio.c | 2 ++ lib/internal.h | 1 + 4 files changed, 6 insertions(+) diff --git a/generator/states-reply-simple.c b/generator/states-reply-simple.c index 12536e0..935f6d2 100644 --- a/generator/states-reply-simple.c +++ b/generator/states-reply-simple.c @@ -40,6 +40,7 @@ if (cmd->error == 0 && cmd->type == NBD_CMD_READ) { h->rbuf = cmd->data; h->rlen = cmd->count; + cmd->data_seen = true; SET_NEXT_STATE (%RECV_READ_PAYLOAD); } else { diff --git a/generator/states-reply-structured.c b/generator/states-reply-structured.c index 9bb165b..6740400 100644 --- a/generator/states-reply-structured.c +++ b/generator/states-reply-structured.c @@ -269,6 +269,7 @@ return -1; } assert (cmd->data); + cmd->data_seen = true; /* Length of the data following. */ length -= 8; @@ -331,6 +332,7 @@ return -1; } assert (cmd->data); + cmd->data_seen = true; /* Is the data within bounds? */ if (offset < cmd->offset) { diff --git a/lib/aio.c b/lib/aio.c index 52e8892..7fb0fdf 100644 --- a/lib/aio.c +++ b/lib/aio.c @@ -73,6 +73,8 @@ nbd_unlocked_aio_command_completed (struct nbd_handle *h, type = cmd->type; error = cmd->error; + if (type == NBD_CMD_READ && !cmd->data_seen && !error) + error = EIO; /* Retire it from the list and free it. */ if (prev_cmd != NULL) diff --git a/lib/internal.h b/lib/internal.h index 6fde06c..1f8f789 100644 --- a/lib/internal.h +++ b/lib/internal.h @@ -242,6 +242,7 @@ struct command_in_flight { uint32_t count; void *data; /* Buffer for read/write, opaque for block status */ extent_fn extent_fn; + bool data_seen; /* For read, true if at least one data chunk seen */ uint32_t error; /* Local errno value */ }; -- 2.20.1 _______________________________________________ Libguestfs mailing list Libguestfs(a)redhat.com https://www.redhat.com/mailman/listinfo/libguestfs