Re: [Libguestfs] [libnbd PATCH 4/4] states: Use RESYNC to handle more structured reply server bugs

Again, compliant servers will never trip over to the new state; but an easy way to demonstrate this in action is with a temporary patch to nbdkit:

@@ -222,13 +181,25 @@ STATE_MACHINE { return 0; case 0: length = be32toh (h->sbuf.sr.structured_reply.length); + assert (length >= sizeof h->sbuf.sr.payload.error.error.error); + assert (cmd); + + if (length < sizeof h->sbuf.sr.payload.error.error) { + resync: + /* Favor the error packet's errno over RESYNC's EPROTO. */ + error = be32toh (h->sbuf.sr.payload.error.error.error); + if (cmd->error = 0)

+ cmd->error = nbd_internal_errno_of_nbd_error (error); + h->rbuf = NULL; + h->rlen = length - MIN (length, sizeof h->sbuf.sr.payload.error.error); + SET_NEXT_STATE (%RESYNC); + return 0; + } + msglen = be16toh (h->sbuf.sr.payload.error.error.len);

@@ -257,24 +228,21 @@ STATE_MACHINE { debug (h, "structured error server message: %.*s", (int) msglen, h->sbuf.sr.payload.error.msg); - /* Special case two specific errors; ignore the tail for all others */ + /* Special case two specific errors; silently ignore tail for all others */ h->rbuf = NULL; h->rlen = length; switch (type) { case NBD_REPLY_TYPE_ERROR: - if (length != 0) { - SET_NEXT_STATE (%.DEAD); - set_error (0, "error payload length too large"); - return 0; - } + if (length != 0) + debug (h, "ignoring unexpected slop after error message, " + "the server may have a bug"); break; case NBD_REPLY_TYPE_ERROR_OFFSET: - if (length != sizeof h->sbuf.sr.payload.error.offset) { - SET_NEXT_STATE (%.DEAD); - set_error (0, "invalid error payload length"); - return 0; - } - h->rbuf = &h->sbuf.sr.payload.error.offset; + if (length == sizeof h->sbuf.sr.payload.error.offset)

+ debug (h, "unable to safely extract error offset, " + "the server may have a bug"); + else + h->rbuf = &h->sbuf.sr.payload.error.offset; break; } SET_NEXT_STATE (%RECV_ERROR_TAIL);