5:12 p.m.
The NBD spec currently requires that block status results be aligned
to any advertised minimum block size (although I'm submitting a patch
in parallel to relax that slightly, to cater to the fact that not all
existing servers comply). And while most plugins and filters _do_
happen to provide aligned results when given aligned inputs, we have
the corner case where an aligned request that results in a 4G+ aligned
response from the plugin is truncated in the server code to an
unaligned answer, which can confuse clients such as 'nbdinfo --map'.
Prior to advertising block sizes, the server didn't have to care about
alignments: the spec only requires aligned results when the client
asks and the server replies with a minimum block size. And even with
the introduction of the .block_size callback, the server still doesn't
have to care for reads or writes: if the client is compliant (or you
use a filter like blocksize or blocksize-policy), the plugin never
sees an unaligned request, and the response will have the same
alignment as the request (because we don't yet implement the ability
to report holes via NBD_CMD_READ - that would require the v3 plugin
API). It is _only_ for extents where we are at the mercy of the
plugin not splitting extents differently than what it advertised as
its minimum block size, which is generally the case, with one obvious
exception: any time the plugin widens an extent and reports 4G or
larger, it is the server (not the plugin) that truncates the request
down to the 32-bit protocol limits.
But since we know that .block_size cannot set a minimum block size
larger than 64k, it is far easier to just truncate the server's
response at a point that is guaranteed to be aligned for all possible
plugins, than to have the server try and munge alignment itself. The
change here is observable in test-eval-extents.sh which was added as
part of the fix for CVE-2025-47711; but fortunately, spec
non-compliance is different than denial of service, so versions of
nbdkit with this bug do NOT represent yet another security lapse, and
the changed test is not any weaker.
Signed-off-by: Eric Blake <eblake(a)redhat.com>
---
server/protocol.c | 12 +++++++++---
tests/test-eval-extents.sh | 6 +++---
2 files changed, 12 insertions(+), 6 deletions(-)
diff --git a/server/protocol.c b/server/protocol.c
index b4b1c162..28b1a554 100644
--- a/server/protocol.c
+++ b/server/protocol.c
@@ -493,13 +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);
+ /* Must not exceed UINT32_MAX. However, if the plugin was
+ * supplying aligned answers, we don't want our truncation to
+ * result in an unaligned point. Rather than tracking whether a
+ * minimumg block size was advertised, it is simpler to just
+ * truncate at 64k shy of 4G, since minimum block size cannot
+ * exceed 64k.
+ */
+ blocks[i].length = length = MIN (e.length, UINT32_MAX & ~0xffff);
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 || length < e.length)
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
index 92b503e6..af0f2885 100755
--- a/tests/test-eval-extents.sh
+++ b/tests/test-eval-extents.sh
@@ -65,7 +65,7 @@ nbdkit eval \
eval-extents.out
cat eval-extents.out
diff -u - eval-extents.out <<EOF
-[4294967294, 1, 1073741824, 2]
-[4294967295, 1]
-[4294967295, 1]
+[4294901760, 1]
+[4294901760, 1]
+[4294901760, 1]
EOF
--
2.49.0