diff options
author | Eric Blake <eblake@redhat.com> | 2023-06-08 08:56:36 -0500 |
---|---|---|
committer | Eric Blake <eblake@redhat.com> | 2023-07-19 15:26:13 -0500 |
commit | 70fa99f445a6fabe4b46f188cc665cd469cd8293 (patch) | |
tree | 7a258a28ea076d84ad7092edef18f9756d9c2002 /nbd | |
parent | 8cb98a725e7397c9de25ebd77c00b1d5f2d8351e (diff) |
nbd/client: Add safety check on chunk payload length
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>
Diffstat (limited to 'nbd')
-rw-r--r-- | nbd/client.c | 12 |
1 files changed, 12 insertions, 0 deletions
diff --git a/nbd/client.c b/nbd/client.c index ea3590ca3d..1b5569556f 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; } |