On Mon, Sep 18, 2023 at 02:31:28PM -0500, Eric Blake wrote:
If a server replies to a block status command with an invalid length
in NBD_REPLY_TYPE_BLOCK_STATUS_EXT, we correctly detect the server's
error, but fail to mark that we've consumed enough data off the wire
to resync back to the server's next reply. Symptoms seen during a
fuzzing run:
│ $ ./fuzzing/libnbd-fuzz-wrapper
│+id\:000001\,sig\:06\,src\:000396\,time\:16888628\,execs\:54159419\,op\:havoc\,rep\:4
│ libnbd-fuzz-wrapper: generator/states-reply-chunk.c:698:
enter_STATE_REPLY_CHUNK_REPLY_FINISH:
│+Assertion `h->payload_left == 0' failed.
│ Aborted (core dumped)
│ read: Connection reset by peer
Appears to be a casualty of rebasing: I added h->payload_left
verification fairly late in the game, then floated it earlier in the
series, and missed a spot where I added a state machine jump to RESYNC
without having updated h->payload_left. An audit of all other
assignments to h->rlen in that file was able to find corresponding
assignments to h->payload_left (often the next statement, but
sometimes split across states based on what made the next state easier
to code).
Requires a non-compliant server, but I was able to come up with a
one-line tweak to pending qemu patches that could trigger it. Not
creating a CVE as it only appears in unstable releases.
Fixes: ab992766cd ("block_status: Accept 64-bit extents during block status")
Thanks: Richard W.M. Jones <rjones(a)redhat.com>
Signed-off-by: Eric Blake <eblake(a)redhat.com>
---
I'm investigating another crash that Rich sent me off-list, but I
suspect it will be a similar non-CVE situation caused by my recent
64-bit extension patches.
I'll wait to apply this one for just a bit more, in case I can get the
corpus file or two from Rich's fuzzing run to add to
fuzzing/testcase_dir.
generator/states-reply-chunk.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/generator/states-reply-chunk.c b/generator/states-reply-chunk.c
index 2cebe456..a5d3aefe 100644
--- a/generator/states-reply-chunk.c
+++ b/generator/states-reply-chunk.c
@@ -476,6 +476,7 @@ REPLY.CHUNK_REPLY.RECV_BS_HEADER:
if (h->bs_count != be32toh (h->sbuf.reply.payload.bs_hdr_64.count)) {
h->rbuf = NULL;
h->rlen = h->payload_left;
+ h->payload_left = 0;
SET_NEXT_STATE (%RESYNC);
return 0;
}
ACK, thanks.
Rich.
--
Richard Jones, Virtualization Group, Red Hat
http://people.redhat.com/~rjones
Read my programming and virtualization blog:
http://rwmj.wordpress.com
nbdkit - Flexible, fast NBD server with plugins
https://gitlab.com/nbdkit/nbdkit