[Libguestfs] [libnbd PATCH 6/7] states: Give up on oversized reply length

Any server that sends a reply to NBD_OPT_* or NBD_CMD_* longer than the maximum length we are willing to use for NBD_CMD_{READ,WRITE} is probably broken; it is not worth waiting for read() to wait for that many bytes to arrive from the server to bring the connection back into sync for the next operation, so just declare it dead. It may be possible to provoke nbdkit into being such a server when replying to NBD_CMD_BLOCK_STATUS on a disk that reports alternating extents, but if so, we should patch nbdkit to truncate before sending that large of a chunk. --- generator/states-newstyle.c | 7 +++++-- generator/states-reply-structured.c | 13 +++++++++++++ 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/generator/states-newstyle.c b/generator/states-newstyle.c index e86282b..369df0f 100644 --- a/generator/states-newstyle.c +++ b/generator/states-newstyle.c @@ -64,9 +64,12 @@ prepare_for_reply_payload (struct nbd_handle *h, uint32_t opt) /* Read the following payload if it is short enough to fit in the * static buffer. If it's too long, skip it. */ - /* XXX Move to DEAD if len > MAX_REQUEST_SIZE */ len = be32toh (h->sbuf.or.option_reply.replylen); - if (len <= maxpayload) + if (len > MAX_REQUEST_SIZE) { + set_error (0, "handshake: invalid option reply length"); + return -1; + } + else if (len <= maxpayload) h->rbuf = &h->sbuf.or.payload; else h->rbuf = NULL; diff --git a/generator/states-reply-structured.c b/generator/states-reply-structured.c index 694c282..06eedb4 100644 --- a/generator/states-reply-structured.c +++ b/generator/states-reply-structured.c @@ -68,6 +68,19 @@ return -1; } + /* Reject a server that replies with too much information, but don't + * reject a single structured reply to NBD_CMD_READ on the largest + * size we were willing to send. The most likely culprit is a server + * that replies with block status with way too many extents, but any + * oversized reply is going to take long enough to resync that it is + * not worth keeping the connection alive. + */ + if (length > MAX_REQUEST_SIZE + sizeof h->sbuf.sr.payload.offset_data) { + set_error (0, "invalid server reply length"); + SET_NEXT_STATE (%.DEAD); + return -1; + } + if (NBD_REPLY_TYPE_IS_ERR (type)) { if (length < sizeof h->sbuf.sr.payload.error) { SET_NEXT_STATE (%.DEAD); -- 2.20.1