[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[PULL 13/14] nbd/client: Add safety check on chunk payload length
From: |
Eric Blake |
Subject: |
[PULL 13/14] nbd/client: Add safety check on chunk payload length |
Date: |
Wed, 19 Jul 2023 15:27:50 -0500 |
Our existing use of structured replies either reads into a qiov capped
at 32M (NBD_CMD_READ) or caps allocation to 1000 bytes (see
NBD_MAX_MALLOC_PAYLOAD in block/nbd.c). But the existing length
checks are rather late; if we encounter a buggy (or malicious) server
that sends a super-large payload length, we should drop the connection
right then rather than assuming the layer on top will be careful.
This becomes more important when we permit 64-bit lengths which are
even more likely to have the potential for attempted denial of service
abuse.
Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Message-ID: <20230608135653.2918540-8-eblake@redhat.com>
---
nbd/client.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/nbd/client.c b/nbd/client.c
index ea3590ca3d0..1b5569556fe 100644
--- a/nbd/client.c
+++ b/nbd/client.c
@@ -1413,6 +1413,18 @@ static int nbd_receive_structured_reply_chunk(QIOChannel
*ioc,
chunk->cookie = be64_to_cpu(chunk->cookie);
chunk->length = be32_to_cpu(chunk->length);
+ /*
+ * Because we use BLOCK_STATUS with REQ_ONE, and cap READ requests
+ * at 32M, no valid server should send us payload larger than
+ * this. Even if we stopped using REQ_ONE, sane servers will cap
+ * the number of extents they return for block status.
+ */
+ if (chunk->length > NBD_MAX_BUFFER_SIZE + sizeof(NBDStructuredReadData)) {
+ error_setg(errp, "server chunk %" PRIu32 " (%s) payload is too long",
+ chunk->type, nbd_rep_lookup(chunk->type));
+ return -EINVAL;
+ }
+
return 0;
}
--
2.41.0
- [PULL 01/14] qemu-nbd: pass structure into nbd_client_thread instead of plain char*, (continued)
- [PULL 01/14] qemu-nbd: pass structure into nbd_client_thread instead of plain char*, Eric Blake, 2023/07/19
- [PULL 06/14] qemu-nbd: make verbose bool and local variable in main(), Eric Blake, 2023/07/19
- [PULL 03/14] qemu-nbd: properly report error if qemu_daemon() is failed, Eric Blake, 2023/07/19
- [PULL 05/14] qemu-nbd: handle dup2() error when qemu-nbd finished setup process, Eric Blake, 2023/07/19
- [PULL 04/14] qemu-nbd: properly report error on error in dup2() after qemu_daemon(), Eric Blake, 2023/07/19
- [PULL 02/14] qemu-nbd: fix regression with qemu-nbd --fork run over ssh, Eric Blake, 2023/07/19
- [PULL 12/14] nbd/client: Simplify cookie vs. index computation, Eric Blake, 2023/07/19
- [PULL 07/14] nbd/client: Use smarter assert, Eric Blake, 2023/07/19
- [PULL 09/14] nbd/server: Prepare for alternate-size headers, Eric Blake, 2023/07/19
- [PULL 08/14] nbd: Consistent typedef usage in header, Eric Blake, 2023/07/19
- [PULL 13/14] nbd/client: Add safety check on chunk payload length,
Eric Blake <=
- [PULL 14/14] nbd: Use enum for various negotiation modes, Eric Blake, 2023/07/19
- [PULL 11/14] nbd: s/handle/cookie/ to match NBD spec, Eric Blake, 2023/07/19
- [PULL 10/14] nbd/server: Refactor to pass full request around, Eric Blake, 2023/07/19
- Re: [PULL 00/14] NBD patches for 2023-07-19, Peter Maydell, 2023/07/21