[Libguestfs] [libnbd PATCH] api: Fix block status assertion under set_strict bypass
A compliant server should not send NBD_REPLY_TYPE_BLOCK_STATUS unless
we successfully negotiated a meta context. And our default strictness
settings refuse to let us send NBD_CMD_BLOCK_STATUS unless we
negotiated a meta context. But when you mix non-default settings
(using nbd_set_strict to disable STRICT_COMMANDS) to send a block
status without having negotiated it, coupled with a non-compliant
server that responds with status anyways, we can then hit the
assertion failure where h->meta_valid is not set during the
REPLY.CHUNK_REPLY.RECV_BS_ENTRIES state.
Demonstration of the assertion failure can be done with a quick patch
to nbdkit (here, on top of v1.35.6):
| diff --git i/server/protocol.c w/server/protocol.c
| index cb530e65..6b115d99 100644
| --- i/server/protocol.c
| +++ w/server/protocol.c
| @@ -190,7 +190,7 @@ validate_request (uint16_t cmd, uint16_t flags, uint64_t offset,
uint32_t count,
| }
|
| /* Block status allowed? */
| - if (cmd == NBD_CMD_BLOCK_STATUS) {
| + if (cmd == NBD_CMD_BLOCK_STATUS && 0) {
| if (!conn->structured_replies) {
| nbdkit_error ("invalid request: "
| "%s: structured replies was not negotiated",
| @@ -536,7 +536,7 @@ send_structured_reply_block_status (uint64_t cookie,
| size_t i;
| int r;
|
| - assert (conn->meta_context_base_allocation);
| + // assert (conn->meta_context_base_allocation);
| assert (cmd == NBD_CMD_BLOCK_STATUS);
|
| blocks = extents_to_block_descriptors (extents, flags, count, offset,
plus this sequence:
$ patched/nbdkit memory 1M
$ ./run nbdsh --opt-mode -u nbd://localhost
nbd> h.set_request_meta_context(False)
nbd> h.set_export_name('a')
nbd> def c(arg):
... pass
...
nbd> h.opt_set_meta_context_queries(['base:allocation'], c)
1
nbd> h.set_export_name('')
nbd> h.opt_go()
nbd> h.set_strict_mode(0)
nbd> h.block_status(1024*1024, 0, c)
nbdsh: generator/states-reply-chunk.c:425: enter_STATE_REPLY_CHUNK_REPLY_RECV_BS_ENTRIES:
Assertion `h->meta_valid' failed.
Aborted (core dumped)
As this is not a typical setup, and is trivially avoided by sticking
to default settings (or more safely, by using TLS connections to
trusted servers that don't reply to a spurious block status call),
this did not warrant a CVE. However, since it is a case where a
server's reply can prompt a libnbd denial of service crash, I will be
sending a security announcement and upcoming patch be to the
libnbd-security man page once backports are in place.
Fixes: 55b09667 ("api: Fix nbd_can_meta_context if NBD_OPT_SET_META_CONTEXT
fails", v1.15.3)
Signed-off-by: Eric Blake <eblake(a)redhat.com>
---
generator/states-reply-chunk.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/generator/states-reply-chunk.c b/generator/states-reply-chunk.c
index 02c65414..26b8a6b0 100644
--- a/generator/states-reply-chunk.c
+++ b/generator/states-reply-chunk.c
@@ -101,6 +101,7 @@ REPLY.CHUNK_REPLY.START:
case NBD_REPLY_TYPE_BLOCK_STATUS:
if (cmd->type != NBD_CMD_BLOCK_STATUS ||
+ !h->meta_valid || h->meta_contexts.len == 0 ||
length < 12 || ((length-4) & 7) != 0)
goto resync;
assert (CALLBACK_IS_NOT_NULL (cmd->cb.fn.extent));
--
2.41.0