[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v5 04/17] nbd: Prepare for 64-bit request effect lengths
From: |
Eric Blake |
Subject: |
Re: [PATCH v5 04/17] nbd: Prepare for 64-bit request effect lengths |
Date: |
Mon, 28 Aug 2023 15:56:10 -0500 |
User-agent: |
NeoMutt/20230517 |
On Thu, Aug 10, 2023 at 12:36:51PM -0500, Eric Blake wrote:
> Widen the length field of NBDRequest to 64-bits, although we can
> assert that all current uses are still under 32 bits: either because
> of NBD_MAX_BUFFER_SIZE which is even smaller (and where size_t can
> still be appropriate, even on 32-bit platforms), or because nothing
> ever puts us into NBD_MODE_EXTENDED yet (and while future patches will
> allow larger transactions, the lengths in play here are still capped
> at 32-bit). Thus no semantic change.
>
> Signed-off-by: Eric Blake <eblake@redhat.com>
> ---
>
> v5: tweak commit message, adjust a few more spots [Vladimir].
>
> v4: split off enum changes to earlier patches [Vladimir]
> ---
> +++ b/include/block/nbd.h
> @@ -71,8 +71,8 @@ typedef enum NBDMode {
> */
> typedef struct NBDRequest {
> uint64_t cookie;
> - uint64_t from;
> - uint32_t len;
> + uint64_t from; /* Offset touched by the command */
> + uint64_t len; /* Effect length; 32 bit limit without extended headers
> */
Despite using unsigned types here...
> +++ b/nbd/server.c
> @@ -1441,7 +1441,7 @@ static int coroutine_fn nbd_receive_request(NBDClient
> *client, NBDRequest *reque
> request->type = lduw_be_p(buf + 6);
> request->cookie = ldq_be_p(buf + 8);
> request->from = ldq_be_p(buf + 16);
> - request->len = ldl_be_p(buf + 24);
> + request->len = ldl_be_p(buf + 24); /* widen 32 to 64 bits */
...this code has a nasty bug in that ldl_be_p() returns int instead of
an unsigned type, so it sign extends, breaking any client that
requests a value larger than 2G but still less than 4G. As there are
still a few patches needing review, I'll go ahead and post a v6 with
the obvious fix folded in.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc.
Virtualization: qemu.org | libguestfs.org
- [PATCH v5 00/17] qemu patches for 64-bit NBD extensions, Eric Blake, 2023/08/10
- [PATCH v5 01/17] nbd: Replace bool structured_reply with mode enum, Eric Blake, 2023/08/10
- [PATCH v5 02/17] nbd/client: Pass mode through to nbd_send_request, Eric Blake, 2023/08/10
- [PATCH v5 06/17] nbd/server: Support a request payload, Eric Blake, 2023/08/10
- [PATCH v5 03/17] nbd: Add types for extended headers, Eric Blake, 2023/08/10
- [PATCH v5 04/17] nbd: Prepare for 64-bit request effect lengths, Eric Blake, 2023/08/10
- Re: [PATCH v5 04/17] nbd: Prepare for 64-bit request effect lengths,
Eric Blake <=
- [PATCH v5 11/17] nbd/client: Plumb errp through nbd_receive_replies, Eric Blake, 2023/08/10
- [PATCH v5 05/17] nbd/server: Refactor handling of command sanity checks, Eric Blake, 2023/08/10
- [PATCH v5 09/17] nbd/server: Support 64-bit block status, Eric Blake, 2023/08/10
- [PATCH v5 14/17] nbd/client: Request extended headers during negotiation, Eric Blake, 2023/08/10
- [PATCH v5 13/17] nbd/client: Accept 64-bit block status chunks, Eric Blake, 2023/08/10
- [PATCH v5 07/17] nbd/server: Prepare to receive extended header requests, Eric Blake, 2023/08/10
- [PATCH v5 08/17] nbd/server: Prepare to send extended header replies, Eric Blake, 2023/08/10
- [PATCH v5 17/17] nbd/server: Add FLAG_PAYLOAD support to CMD_BLOCK_STATUS, Eric Blake, 2023/08/10
- [PATCH v5 10/17] nbd/server: Enable initial support for extended headers, Eric Blake, 2023/08/10