[Libguestfs] Re: nbdkit crashes on attempt to retrieve extent with len of 2^32-1
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