[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[libnbd PATCH v2 06/23] states: Break deadlock if server goofs on extend
From: |
Eric Blake |
Subject: |
[libnbd PATCH v2 06/23] states: Break deadlock if server goofs on extended replies |
Date: |
Mon, 14 Nov 2022 16:51:41 -0600 |
One of the benefits of extended replies is that we can do a
fixed-length read for the entire header of every server reply, which
is fewer syscalls than the split-read approach required by structured
replies. But one of the drawbacks of doing a large read is that if
the server is non-compliant (not a problem for normal servers, but
something I hit rather more than I'd like to admit while developing
extended header support in servers), nbd_pwrite() and friends will
deadlock if the server replies with the wrong header. Add in some
code to catch that failure mode and move the state machine to DEAD
sooner, to make it easier to diagnose the fault in the server.
Unlike in the case of an unexpected simply reply from a structured
server (where we never over-read, and can therefore commit b31e7bac
can merely fail the command with EPROTO and successfully move on to
the next server reply), in this case we really do have to move to
DEAD: in addition to having already read the 16 or 20 bytes that the
server sent in its (short) reply for this command, we may have already
read the initial bytes of the server's next reply, but we have no way
to push those extra bytes back onto our read stream for parsing on our
next pass through the state machine.
---
generator/states-reply.c | 18 +++++++++++++++++-
1 file changed, 17 insertions(+), 1 deletion(-)
diff --git a/generator/states-reply.c b/generator/states-reply.c
index dde23b39..e89e9019 100644
--- a/generator/states-reply.c
+++ b/generator/states-reply.c
@@ -109,7 +109,23 @@ REPLY.START:
REPLY.RECV_REPLY:
switch (recv_into_rbuf (h)) {
case -1: SET_NEXT_STATE (%.DEAD); return 0;
- case 1: SET_NEXT_STATE (%.READY); return 0;
+ case 1: SET_NEXT_STATE (%.READY);
+ /* Special case: if we have a short read, but got at least far
+ * enough to decode the magic number, we can check if the server
+ * is matching our expectations. This lets us avoid deadlocking if
+ * a buggy server sends only 16 bytes of a simple reply, and is
+ * waiting for our next command, while we are blocked waiting for
+ * the server to send 32 bytes of an extended reply.
+ */
+ if (h->extended_headers &&
+ (char *) h->rbuf >= (char *) &h->sbuf.reply.hdr.extended.flags) {
+ uint32_t magic = be32toh (h->sbuf.reply.hdr.extended.magic);
+ if (magic != NBD_EXTENDED_REPLY_MAGIC) {
+ SET_NEXT_STATE (%.DEAD); /* We've probably lost synchronization. */
+ set_error (0, "invalid or unexpected reply magic 0x%" PRIx32, magic);
+ }
+ }
+ return 0;
case 0: SET_NEXT_STATE (%CHECK_SIMPLE_OR_STRUCTURED_REPLY);
}
return 0;
--
2.38.1
- [libnbd PATCH v2 11/23] api: Add several functions for controlling extended headers, (continued)
- [libnbd PATCH v2 11/23] api: Add several functions for controlling extended headers, Eric Blake, 2022/11/14
- [libnbd PATCH v2 03/23] protocol: Add definitions for extended headers, Eric Blake, 2022/11/14
- [libnbd PATCH v2 02/23] internal: Refactor layout of replies in sbuf, Eric Blake, 2022/11/14
- [libnbd PATCH v2 04/23] states: Prepare to send 64-bit requests, Eric Blake, 2022/11/14
- [libnbd PATCH v2 14/23] info: Expose extended-headers support through nbdinfo, Eric Blake, 2022/11/14
- [libnbd PATCH v2 08/23] block_status: Track 64-bit extents internally, Eric Blake, 2022/11/14
- [libnbd PATCH v2 01/23] block_status: Refactor array storage, Eric Blake, 2022/11/14
- [libnbd PATCH v2 05/23] states: Prepare to receive 64-bit replies, Eric Blake, 2022/11/14
- [libnbd PATCH v2 18/23] generator: Actually request extended headers, Eric Blake, 2022/11/14
- [libnbd PATCH v2 20/23] interop: Add test of 64-bit block status, Eric Blake, 2022/11/14
- [libnbd PATCH v2 06/23] states: Break deadlock if server goofs on extended replies,
Eric Blake <=
- [libnbd PATCH v2 17/23] ocaml: Add example for 64-bit extents, Eric Blake, 2022/11/14
- [libnbd PATCH v2 07/23] generator: Add struct nbd_extent in prep for 64-bit extents, Eric Blake, 2022/11/14
- [PATCH v2 00/15] qemu patches for 64-bit NBD extensions, Eric Blake, 2022/11/14
- [PATCH v2 15/15] RFC: nbd/server: Send 64-bit hole chunk, Eric Blake, 2022/11/14
- [PATCH v2 14/15] RFC: nbd/client: Accept 64-bit hole chunks, Eric Blake, 2022/11/14
- [PATCH v2 06/15] nbd/server: Refactor to pass full request around, Eric Blake, 2022/11/14
- [PATCH v2 11/15] nbd/client: Request extended headers during negotiation, Eric Blake, 2022/11/14
- [PATCH v2 12/15] nbd/server: Prepare for per-request filtering of BLOCK_STATUS, Eric Blake, 2022/11/14
- [PATCH v2 13/15] nbd/server: Add FLAG_PAYLOAD support to CMD_BLOCK_STATUS, Eric Blake, 2022/11/14
- [PATCH v2 10/15] nbd/client: Accept 64-bit block status chunks, Eric Blake, 2022/11/14