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 <ebl...@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
