Re: [PATCH v6 04/17] nbd: Prepare for 64-bit request effect lengths

[Vladimir Sementsov-Ogievskiy]

On 29.08.23 20:58, 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).  There are no semantic changes, other than a typo fix in a
> couple of error messages.
>
> Signed-off-by: Eric Blake <eblake@redhat.com>
> ---
>
> v6: fix sign extension bug
>
> v5: tweak commit message, adjust a few more spots [Vladimir]
>
> v4: split off enum changes to earlier patches [Vladimir]

[..]

> --- a/nbd/server.c
> +++ b/nbd/server.c
> @@ -1165,7 +1165,7 @@ static int nbd_negotiate_options(NBDClient *client, Error 
> **errp)
>           client->optlen = length;
>
>           if (length > NBD_MAX_BUFFER_SIZE) {
> -            error_setg(errp, "len (%" PRIu32" ) is larger than max len (%u)",
> +            error_setg(errp, "len (%" PRIu32 ") is larger than max len (%u)",
>                          length, NBD_MAX_BUFFER_SIZE);
>               return -EINVAL;
>           }
> @@ -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    = (uint32_t)ldl_be_p(buf + 24); /* widen 32 to 64 bits */

should it be "(uint64_t)" ?

>       trace_nbd_receive_request(magic, request->flags, request->type,
>                                 request->from, request->len);
> @@ -1899,7 +1899,7 @@ static int coroutine_fn 
> nbd_co_send_simple_reply(NBDClient *client,
>                                                    NBDRequest *request,
>                                                    uint32_t error,
>                                                    void *data,
> -                                                 size_t len,
> +                                                 uint64_t len,
>                                                    Error **errp)
>   {
>       NBDSimpleReply reply;
> @@ -1910,6 +1910,7 @@ static int coroutine_fn 
> nbd_co_send_simple_reply(NBDClient *client,
>       };
>
>       assert(!len || !nbd_err);
> +    assert(len <= NBD_MAX_STRING_SIZE);

NBD_MAX_BUFFER_SIZE ?

with that fixed:
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>



--
Best regards,
Vladimir