[nbdkit PATCH v2 0/2] Fix two assertion failures on large block size
Still waiting on Red Hat's security team to decide if these get CVE designations, but at this point, we consider the impact to be low enough severity (easy to avoid if your server rejects malicious clients by the use of TLS) and related enough that there is no longer any need to embargo the second one. I'll wait a bit longer to apply, to provide time to update the subject lines according to whether we get CVEs assigned. Eric Blake (2): server: Fix off-by-one for maximum block_status length [CVE-XXX] blocksize: Fix 32-bit overflow in .extents [CVE-XXXX] tests/Makefile.am | 4 ++ server/protocol.c | 2 +- filters/blocksize/blocksize.c | 5 +- tests/test-blocksize-extents-overflow.sh | 83 ++++++++++++++++++++++++ tests/test-eval-extents.sh | 71 ++++++++++++++++++++ 5 files changed, 162 insertions(+), 3 deletions(-) create mode 100755 tests/test-blocksize-extents-overflow.sh create mode 100755 tests/test-eval-extents.sh -- 2.49.0
There has been an off-by-one bug in the code for .extents since the introduction of that callback. Remember, internally the code allows plugins to report on extents with 64-bit lengths, but the protocol only supports 32-bit block status calls (nbdkit will need to create plugin version 3 before it can support NBD's newer 64-bit block status). As such, the server loop intentionally truncates a plugin's large extent to 2**32-1 bytes. But in the process of checking whether the loop should exit early, or if any additional extents should be reported to the client, the server used 'pos > offset+count' instead of >=, which is one byte too far. If the client has requested exactly 2**32-1 bytes, and the plugin's first extent has that same length, the code erroneously proceeds on to the plugin's second extent. Worse, if the plugin's first extent has 2**32 bytes or more, it was truncated to 2**31-1 bytes, but not completely handled, and the failure to exit the loop early means that the server then fails the assertion: nbdkit: ../../server/protocol.c:505: extents_to_block_descriptors: Assertion `e.length <= length' failed. The single-byte fix addresses both symptoms, while the added test demonstrates both when run on older nbdkit (the protocol violation when the plugin returns 2**32-1 bytes in the first extent, and the assertion failure when the plugin returns 2**32 or more bytes in the first extent). The problem can only be triggered by a client request for 2**32-1 bytes; anything smaller is immune. The problam also does not occur for plugins that do not return extents information beyond the client's request, or if the first extent is smaller than the client's request. The ability 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 require the use of TLS, then you can ensure that you only have trusted clients that won't trigger a block status call of length 2**32-1 bytes. Also, you can use "--filter=blocksize-policy blocksize-minimum=512" to reject block status attempts from clients that are not sector-aligned. Fixes: 26455d45 ('server: protocol: Implement Block Status "base:allocation".', v1.11.10) Reported-by: Nikolay Ivanets <stenavin@gmail.com> Signed-off-by: Eric Blake <eblake@redhat.com> Acked-by: Richard W.M. Jones <rjones@redhat.com> --- tests/Makefile.am | 2 ++ server/protocol.c | 2 +- tests/test-eval-extents.sh | 71 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 74 insertions(+), 1 deletion(-) create mode 100755 tests/test-eval-extents.sh diff --git a/tests/Makefile.am b/tests/Makefile.am index 13c0457c..ffe42c78 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -881,6 +881,7 @@ TESTS += \ test-eval.sh \ test-eval-file.sh \ test-eval-exports.sh \ + test-eval-extents.sh \ test-eval-cache.sh \ test-eval-dump-plugin.sh \ test-eval-disconnect.sh \ @@ -889,6 +890,7 @@ EXTRA_DIST += \ test-eval.sh \ test-eval-file.sh \ test-eval-exports.sh \ + test-eval-extents.sh \ test-eval-cache.sh \ test-eval-dump-plugin.sh \ test-eval-disconnect.sh \ diff --git a/server/protocol.c b/server/protocol.c index d428bfc8..b4b1c162 100644 --- a/server/protocol.c +++ b/server/protocol.c @@ -499,7 +499,7 @@ extents_to_block_descriptors (struct nbdkit_extents *extents, (*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 diff --git a/tests/test-eval-extents.sh b/tests/test-eval-extents.sh new file mode 100755 index 00000000..adc2b5b0 --- /dev/null +++ b/tests/test-eval-extents.sh @@ -0,0 +1,71 @@ +#!/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. + +source ./functions.sh +set -e +set -x + +requires_run +requires_plugin eval +requires_nbdsh_uri +requires nbdsh --base-allocation --version + +files="eval-extents.out" +rm -f $files +cleanup_fn rm -f $files + +# Trigger an off-by-one bug introduced in v1.11.10 and fixed in v1.43.6 +export script=' +def f(context, offset, extents, status): + print(extents) + +# First, probe where the server should return 2 extents. +h.block_status(2**32-1, 2, f) + +# Next, probe where the server has exactly 2**32-1 bytes in its first extent. +h.block_status(2**32-1, 1, f) + +# Now, probe where the first extent has to be truncated. +h.block_status(2**32-1, 0, f) +' +nbdkit eval \ + get_size='echo 5G' \ + pread='dd if=/dev/zero count=$3 iflag=count_bytes' \ + extents='echo 0 4G 1; echo 4G 1G 2' \ + --run 'nbdsh --base-allocation --uri "$uri" -c "$script"' \ + > eval-extents.out +cat eval-extents.out +cat <<EOF | diff -u - eval-extents.out +[4294967294, 1, 1073741824, 2] +[4294967295, 1] +[4294967295, 1] +EOF -- 2.49.0
On Wed, Apr 23, 2025 at 04:06:45PM -0500, Eric Blake wrote:
There has been an off-by-one bug in the code for .extents since the introduction of that callback. Remember, internally the code allows plugins to report on extents with 64-bit lengths, but the protocol only supports 32-bit block status calls (nbdkit will need to create plugin version 3 before it can support NBD's newer 64-bit block status). As such, the server loop intentionally truncates a plugin's large extent to 2**32-1 bytes. But in the process of checking whether the loop should exit early, or if any additional extents should be reported to the client, the server used 'pos > offset+count' instead of >=, which is one byte too far. If the client has requested exactly 2**32-1 bytes, and the plugin's first extent has that same length, the code erroneously proceeds on to the plugin's second extent. Worse, if the plugin's first extent has 2**32 bytes or more, it was truncated to 2**31-1 bytes, but not completely handled, and the failure to exit the loop early means that the server then fails the assertion:
nbdkit: ../../server/protocol.c:505: extents_to_block_descriptors: Assertion `e.length <= length' failed.
The single-byte fix addresses both symptoms, while the added test demonstrates both when run on older nbdkit (the protocol violation when the plugin returns 2**32-1 bytes in the first extent, and the assertion failure when the plugin returns 2**32 or more bytes in the first extent).
The problem can only be triggered by a client request for 2**32-1 bytes; anything smaller is immune. The problam also does not occur for plugins that do not return extents information beyond the client's request, or if the first extent is smaller than the client's request.
The ability 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 require the use of TLS, then you can ensure that you only have trusted clients that won't trigger a block status call of length 2**32-1 bytes. Also, you can use "--filter=blocksize-policy blocksize-minimum=512" to reject block status attempts from clients that are not sector-aligned.
Fixes: 26455d45 ('server: protocol: Implement Block Status "base:allocation".', v1.11.10) Reported-by: Nikolay Ivanets <stenavin@gmail.com> Signed-off-by: Eric Blake <eblake@redhat.com> Acked-by: Richard W.M. Jones <rjones@redhat.com> --- tests/Makefile.am | 2 ++ server/protocol.c | 2 +- tests/test-eval-extents.sh | 71 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 74 insertions(+), 1 deletion(-) create mode 100755 tests/test-eval-extents.sh
diff --git a/tests/Makefile.am b/tests/Makefile.am index 13c0457c..ffe42c78 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -881,6 +881,7 @@ TESTS += \ test-eval.sh \ test-eval-file.sh \ test-eval-exports.sh \ + test-eval-extents.sh \ test-eval-cache.sh \ test-eval-dump-plugin.sh \ test-eval-disconnect.sh \ @@ -889,6 +890,7 @@ EXTRA_DIST += \ test-eval.sh \ test-eval-file.sh \ test-eval-exports.sh \ + test-eval-extents.sh \ test-eval-cache.sh \ test-eval-dump-plugin.sh \ test-eval-disconnect.sh \ diff --git a/server/protocol.c b/server/protocol.c index d428bfc8..b4b1c162 100644 --- a/server/protocol.c +++ b/server/protocol.c @@ -499,7 +499,7 @@ extents_to_block_descriptors (struct nbdkit_extents *extents, (*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 diff --git a/tests/test-eval-extents.sh b/tests/test-eval-extents.sh new file mode 100755 index 00000000..adc2b5b0 --- /dev/null +++ b/tests/test-eval-extents.sh @@ -0,0 +1,71 @@ +#!/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. + +source ./functions.sh +set -e +set -x + +requires_run +requires_plugin eval +requires_nbdsh_uri +requires nbdsh --base-allocation --version + +files="eval-extents.out" +rm -f $files +cleanup_fn rm -f $files + +# Trigger an off-by-one bug introduced in v1.11.10 and fixed in v1.43.6 +export script=' +def f(context, offset, extents, status): + print(extents) + +# First, probe where the server should return 2 extents. +h.block_status(2**32-1, 2, f) + +# Next, probe where the server has exactly 2**32-1 bytes in its first extent. +h.block_status(2**32-1, 1, f) + +# Now, probe where the first extent has to be truncated. +h.block_status(2**32-1, 0, f) +' +nbdkit eval \ + get_size='echo 5G' \ + pread='dd if=/dev/zero count=$3 iflag=count_bytes' \ + extents='echo 0 4G 1; echo 4G 1G 2' \ + --run 'nbdsh --base-allocation --uri "$uri" -c "$script"' \ + > eval-extents.out +cat eval-extents.out +cat <<EOF | diff -u - eval-extents.out +[4294967294, 1, 1073741824, 2] +[4294967295, 1] +[4294967295, 1] +EOF
^^ I really need to use this 'diff -' trick in a few other places ... Reviewed-by: Richard W.M. Jones <rjones@redhat.com> -- 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
On Wed, Apr 23, 2025 at 10:19:53PM +0100, Richard W.M. Jones wrote:
The problem can only be triggered by a client request for 2**32-1 bytes; anything smaller is immune. The problam also does not occur
s/problam/problem/ (and seeing this made me think to run codespell and fix a number of other typos)
+cat eval-extents.out +cat <<EOF | diff -u - eval-extents.out +[4294967294, 1, 1073741824, 2] +[4294967295, 1] +[4294967295, 1] +EOF
^^ I really need to use this 'diff -' trick in a few other places ...
Useless use of cat. It can be further simplified to: diff -u - eval-extents.out <<EOF expected... EOF Also, I've noticed that we are inconsistent on whether we use 'diff expected actual' or 'diff actual expected'. Personally, I find 'expected actual' to produce nicer output (if expected is wrong because of a false positive, such as due to an intentional change in an error message, the output shows you how to patch the expected so that re-running will work the new desired state).
Reviewed-by: Richard W.M. Jones <rjones@redhat.com>
Thanks; I'll include that before pushing. I'm still awaiting resolution on whether this gets a CVE assigned. -- Eric Blake, Principal Software Engineer Red Hat, Inc. Virtualization: qemu.org | libguestfs.org
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 <eblake@redhat.com> Acked-by: Richard W.M. Jones <rjones@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"' -- 2.49.0
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 <eblake@redhat.com> Acked-by: Richard W.M. Jones <rjones@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 <rjones@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
On Wed, Apr 23, 2025 at 04:06:44PM -0500, Eric Blake via Libguestfs wrote:
Still waiting on Red Hat's security team to decide if these get CVE designations, but at this point, we consider the impact to be low enough severity (easy to avoid if your server rejects malicious clients by the use of TLS) and related enough that there is no longer any need to embargo the second one.
I'll wait a bit longer to apply, to provide time to update the subject lines according to whether we get CVEs assigned.
Eric Blake (2): server: Fix off-by-one for maximum block_status length [CVE-XXX] blocksize: Fix 32-bit overflow in .extents [CVE-XXXX]
These have now been assigned CVE identifiers. CVE-2025-47711 is for the server error with any plugin returning .extents of 4G or more, and CVE-2025-47712 is for the blocksize filter bug on unaligned block status requests near 4G. I am now in the process of applying the patches to mainline and backporting them to branches that are still in active use; I will send a followup mail with tests for vulnerable versions and version numbers/commit ids to be used to avoid the problems, along with a patch to docs/nbdkit-security.pod pointing to that eventual mail. -- Eric Blake, Principal Software Engineer Red Hat, Inc. Virtualization: qemu.org | libguestfs.org
On Thu, May 08, 2025 at 01:51:21PM -0500, Eric Blake via Libguestfs wrote:
On Wed, Apr 23, 2025 at 04:06:44PM -0500, Eric Blake via Libguestfs wrote:
Still waiting on Red Hat's security team to decide if these get CVE designations, but at this point, we consider the impact to be low enough severity (easy to avoid if your server rejects malicious clients by the use of TLS) and related enough that there is no longer any need to embargo the second one.
I'll wait a bit longer to apply, to provide time to update the subject lines according to whether we get CVEs assigned.
Eric Blake (2): server: Fix off-by-one for maximum block_status length [CVE-XXX] blocksize: Fix 32-bit overflow in .extents [CVE-XXXX]
These have now been assigned CVE identifiers. CVE-2025-47711 is for the server error with any plugin returning .extents of 4G or more, and CVE-2025-47712 is for the blocksize filter bug on unaligned block status requests near 4G.
I am now in the process of applying the patches to mainline and backporting them to branches that are still in active use; I will send a followup mail with tests for vulnerable versions and version numbers/commit ids to be used to avoid the problems, along with a patch to docs/nbdkit-security.pod pointing to that eventual mail.
Thanks Eric. The fixes are available in development version 1.43.7 and stable version 1.42.3. Rich. -- 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
On Sat, May 10, 2025 at 10:08:30AM +0100, Richard W.M. Jones wrote:
On Thu, May 08, 2025 at 01:51:21PM -0500, Eric Blake via Libguestfs wrote:
On Wed, Apr 23, 2025 at 04:06:44PM -0500, Eric Blake via Libguestfs wrote:
Still waiting on Red Hat's security team to decide if these get CVE designations, but at this point, we consider the impact to be low enough severity (easy to avoid if your server rejects malicious clients by the use of TLS) and related enough that there is no longer any need to embargo the second one.
I'll wait a bit longer to apply, to provide time to update the subject lines according to whether we get CVEs assigned.
Eric Blake (2): server: Fix off-by-one for maximum block_status length [CVE-XXX] blocksize: Fix 32-bit overflow in .extents [CVE-XXXX]
These have now been assigned CVE identifiers. CVE-2025-47711 is for the server error with any plugin returning .extents of 4G or more, and CVE-2025-47712 is for the blocksize filter bug on unaligned block status requests near 4G.
I am now in the process of applying the patches to mainline and backporting them to branches that are still in active use; I will send a followup mail with tests for vulnerable versions and version numbers/commit ids to be used to avoid the problems, along with a patch to docs/nbdkit-security.pod pointing to that eventual mail.
Thanks Eric. The fixes are available in development version 1.43.7 and stable version 1.42.3.
And now in stable versions 1.40.6 and 1.38.6. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com libguestfs lets you edit virtual machines. Supports shell scripting, bindings from many languages. http://libguestfs.org
On Sat, May 10, 2025 at 10:44:36AM +0100, Richard W.M. Jones via Libguestfs wrote:
On Sat, May 10, 2025 at 10:08:30AM +0100, Richard W.M. Jones wrote:
On Thu, May 08, 2025 at 01:51:21PM -0500, Eric Blake via Libguestfs wrote:
On Wed, Apr 23, 2025 at 04:06:44PM -0500, Eric Blake via Libguestfs wrote:
Still waiting on Red Hat's security team to decide if these get CVE designations, but at this point, we consider the impact to be low enough severity (easy to avoid if your server rejects malicious clients by the use of TLS) and related enough that there is no longer any need to embargo the second one.
I'll wait a bit longer to apply, to provide time to update the subject lines according to whether we get CVEs assigned.
Eric Blake (2): server: Fix off-by-one for maximum block_status length [CVE-XXX] blocksize: Fix 32-bit overflow in .extents [CVE-XXXX]
These have now been assigned CVE identifiers. CVE-2025-47711 is for the server error with any plugin returning .extents of 4G or more, and CVE-2025-47712 is for the blocksize filter bug on unaligned block status requests near 4G.
I am now in the process of applying the patches to mainline and backporting them to branches that are still in active use; I will send a followup mail with tests for vulnerable versions and version numbers/commit ids to be used to avoid the problems, along with a patch to docs/nbdkit-security.pod pointing to that eventual mail.
Thanks Eric. The fixes are available in development version 1.43.7 and stable version 1.42.3.
And now in stable versions 1.40.6 and 1.38.6.
The website is now updated too: https://libguestfs.org/nbdkit-security.1.html Rich. -- 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
participants (2)
-
Eric Blake -
Richard W.M. Jones