On Wed, Apr 23, 2025 at 04:06:46PM -0500, Eric Blake wrote:
> If the original request is larger than 2**32 - minblock, then we were
> calling nbdkit_extents_aligned() with a count that rounded up then
> overflowed to 0 instead of the intended 4G because of overflowing a
> 32-bit type, which in turn causes an assertion failure:
> 
> nbdkit: ../../server/backend.c:814: backend_extents: Assertion 
> `backend_valid_range (c, offset, count)' failed.
> 
> The fix is to force the rounding to be in a 64-bit type from the
> get-go.
> 
> The ability for a well-behaved client to cause the server to die from
> an assertion failure can be used as a denial of service attack against
> other clients.  Mitigations: if you requrire the use of TLS, then you
> can ensure that you only have trusted clients that won't trigger a
> block status call that large.  Also, the problem only occurs when
> using the blocksize filter, although setting the filter's maxlen
> parameter to a smaller value than its default of 2**32-1 does not
> help.
> 
> Fixes: 2680be00 ('blocksize: Fix .extents when plugin changes type within 
> minblock', v1.21.16)
> Signed-off-by: Eric Blake <ebl...@redhat.com>
> Acked-by: Richard W.M. Jones <rjo...@redhat.com>
> ---
>  tests/Makefile.am                        |  2 +
>  filters/blocksize/blocksize.c            |  5 +-
>  tests/test-blocksize-extents-overflow.sh | 83 ++++++++++++++++++++++++
>  3 files changed, 88 insertions(+), 2 deletions(-)
>  create mode 100755 tests/test-blocksize-extents-overflow.sh
> 
> diff --git a/tests/Makefile.am b/tests/Makefile.am
> index ffe42c78..9efb6101 100644
> --- a/tests/Makefile.am
> +++ b/tests/Makefile.am
> @@ -1639,12 +1639,14 @@ test_layers_filter3_la_LIBADD = 
> $(IMPORT_LIBRARY_ON_WINDOWS)
>  TESTS += \
>       test-blocksize.sh \
>       test-blocksize-extents.sh \
> +     test-blocksize-extents-overflow.sh \
>       test-blocksize-default.sh \
>       test-blocksize-sharding.sh \
>       $(NULL)
>  EXTRA_DIST += \
>       test-blocksize.sh \
>       test-blocksize-extents.sh \
> +     test-blocksize-extents-overflow.sh \
>       test-blocksize-default.sh \
>       test-blocksize-sharding.sh \
>       $(NULL)
> diff --git a/filters/blocksize/blocksize.c b/filters/blocksize/blocksize.c
> index 09195cea..e5c8b744 100644
> --- a/filters/blocksize/blocksize.c
> +++ b/filters/blocksize/blocksize.c
> @@ -482,8 +482,9 @@ blocksize_extents (nbdkit_next *next,
>      return -1;
>    }
> 
> -  if (nbdkit_extents_aligned (next, MIN (ROUND_UP (count, h->minblock),
> -                                         h->maxlen),
> +  if (nbdkit_extents_aligned (next,
> +                              MIN (ROUND_UP ((uint64_t) count, h->minblock),
> +                                   h->maxlen),
>                                ROUND_DOWN (offset, h->minblock), flags,
>                                h->minblock, extents2, err) == -1)
>      return -1;
> diff --git a/tests/test-blocksize-extents-overflow.sh 
> b/tests/test-blocksize-extents-overflow.sh
> new file mode 100755
> index 00000000..844c3999
> --- /dev/null
> +++ b/tests/test-blocksize-extents-overflow.sh
> @@ -0,0 +1,83 @@
> +#!/usr/bin/env bash
> +# nbdkit
> +# Copyright Red Hat
> +#
> +# Redistribution and use in source and binary forms, with or without
> +# modification, are permitted provided that the following conditions are
> +# met:
> +#
> +# * Redistributions of source code must retain the above copyright
> +# notice, this list of conditions and the following disclaimer.
> +#
> +# * Redistributions in binary form must reproduce the above copyright
> +# notice, this list of conditions and the following disclaimer in the
> +# documentation and/or other materials provided with the distribution.
> +#
> +# * Neither the name of Red Hat nor the names of its contributors may be
> +# used to endorse or promote products derived from this software without
> +# specific prior written permission.
> +#
> +# THIS SOFTWARE IS PROVIDED BY RED HAT AND CONTRIBUTORS ''AS IS'' AND
> +# ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
> +# THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A
> +# PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL RED HAT OR
> +# CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
> +# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
> +# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF
> +# USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
> +# ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
> +# OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT
> +# OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
> +# SUCH DAMAGE.
> +
> +# Demonstrate a fix for a bug where blocksize overflowed 32 bits
> +
> +source ./functions.sh
> +set -e
> +set -x
> +
> +requires_run
> +requires_plugin eval
> +requires_nbdsh_uri
> +requires nbdsh --base-allocation --version
> +
> +# Script a sparse server that requires 512-byte aligned requests.
> +exts='
> +if test $(( ($3|$4) & 511 )) != 0; then
> +  echo "EINVAL request unaligned" 2>&1
> +  exit 1
> +fi
> +echo 0 5G 0
> +'
> +
> +# We also need an nbdsh script to parse all extents, coalescing adjacent
> +# types for simplicity.
> +# FIXME: Once nbdkit plugin version 3 allows 64-bit block extents, run
> +# this test twice, once for each bit size (32-bit needs 2 extents, 64-bit
> +# will get the same result with only 1 extent).
> +export script='
> +size = h.get_size()
> +offs = 0
> +entries = []
> +def f(metacontext, offset, e, err):
> +    global entries
> +    global offs
> +    assert offs == offset
> +    for length, flags in zip(*[iter(e)] * 2):
> +        if entries and flags == entries[-1][1]:
> +            entries[-1] = (entries[-1][0] + length, flags)
> +        else:
> +            entries.append((length, flags))
> +        offs = offs + length
> +
> +# Test a loop over the entire device
> +while offs < size:
> +    len = min(size - offs, 2**32-1)
> +    h.block_status(len, offs, f)
> +assert entries == [(5 * 2**30, 0)]
> +'
> +
> +# Now run everything
> +nbdkit --filter=blocksize eval minblock=512 \
> +       get_size='echo 5G' pread='exit 1' extents="$exts" \
> +       --run 'nbdsh --base-allocation -u "$uri" -c "$script"'

Reviewed-by: Richard W.M. Jones <rjo...@redhat.com>


-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-builder quickly builds VMs from scratch
http://libguestfs.org/virt-builder.1.html
_______________________________________________
Libguestfs mailing list -- guestfs@lists.libguestfs.org
To unsubscribe send an email to guestfs-le...@lists.libguestfs.org

Reply via email to