On Tue, Apr 22, 2025 at 04:05:28PM -0500, Eric Blake via Libguestfs wrote:
> On Tue, Apr 22, 2025 at 03:41:43PM -0500, Eric Blake via Libguestfs wrote:
> > On Tue, Apr 22, 2025 at 10:08:01PM +0300, Nikolay Ivanets wrote:
> > > nbdkit crashes when the client is trying to get extents with len=2^32-1.
> > > Here is client code (nbdsh):
> > >
> > > h.add_meta_context("base:allocation")
> > > h.connect_uri("nbd://localhost:10809/disk0-flat.vmdk")
> >
> > Can you post the disk0-flag.vmdk image (or better, instructions for
> > how to create it?)
> >
> > >
> > > def f(metacontext, offset, e, status):
> > > print(e)
> > >
> > > h.block_status(2**32-2, 0, f)
> > > [4294967295, 0] <--- OK
>
> At any rate, it looks like you have an export that is all data up to this
> length...
>
> > > Server prints:
> > >
> > > nbdkit: file.8: debug: file: extents count=4294967295 offset=0 req_one=0
>
> ...and that you were using the file plugin to read it. So let's see
> what happens if I create a file larger than 4G to try and reproduce:
>
> $ dd if=/dev/urandom of=myfile bs=1G count=5
> 5+0 records in
> 5+0 records out
> 5368709120 bytes (5.4 GB, 5.0 GiB) copied, 15.5435 s, 345 MB/s
> $ ./nbdkit -fv file myfile &
> $ nbdsh --base-allocation --uri nbd://localhost
> <nbd> def f(m, o, e, s):
> ... print(e)
> ...
> <nbd> h.block_status(2**32-2, 0, f)
> [4294967295, 0]
> <nbd> h.block_status(2**32-1, 0, f)
> server crashed
>
> Okay, I can reproduce it with the file plugin, but not with the memory
> plugin. Now working on root cause.
Found it. Off-by-one, looks like it was introduced in commit 26455d45
(v1.11.10), fix is:
diff --git i/server/protocol.c w/server/protocol.c
index d428bfc8..b4b1c162 100644
--- i/server/protocol.c
+++ w/server/protocol.c
@@ -493,19 +493,19 @@ extents_to_block_descriptors (struct nbdkit_extents
*extents,
if (i == 0)
assert (e.offset == offset);
/* Must not exceed UINT32_MAX. */
blocks[i].length = length = MIN (e.length, UINT32_MAX);
blocks[i].status_flags = e.type & 3;
(*nr_blocks)++;
pos += length;
- if (pos > offset + count) /* this must be the last block */
+ if (pos >= offset + count) /* this must be the last block */
break;
/* If we reach here then we must have consumed this whole
* extent. This is currently true because the server only sends
* 32 bit requests, but if we move to 64 bit requests we will
* need to revisit this code so it can split extents into
* multiple blocks. XXX
*/
assert (e.length <= length);
And since this means an arbitrary client can crash the server, we
probably need a CVE. It's already public knowledge, so there is no
embargo.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc.
Virtualization: qemu.org | libguestfs.org
_______________________________________________
Libguestfs mailing list -- guestfs@lists.libguestfs.org
To unsubscribe send an email to guestfs-le...@lists.libguestfs.org
