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 <rjo...@redhat.com> > Signed-off-by: Eric Blake <ebl...@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 _______________________________________________ Libguestfs mailing list Libguestfs@redhat.com https://listman.redhat.com/mailman/listinfo/libguestfs