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\:01\,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
> Signed-off-by: Eric Blake
> ---
>
> 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
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs
