On 08.06.23 22:15, Eric Blake wrote:
On Thu, Jun 08, 2023 at 08:56:52AM -0500, Eric Blake wrote:
> The next commit will add support for the optional extension
> NBD_CMD_FLAG_PAYLOAD during NBD_CMD_BLOCK_STATUS, where the client can
> request that the server only return a subset of negotiated contexts,
> rather than all contexts. To make that task easier, this patch
> populates the list of contexts to return on a per-command basis (for
> now, identical to the full set of negotiated contexts).
>
> Signed-off-by: Eric Blake <eblake(a)redhat.com>
> ---
>
> +++ b/nbd/server.c
> @@ -2491,6 +2491,8 @@ static int coroutine_fn nbd_co_receive_request(NBDRequestData
*req, NBDRequest *
> error_setg(errp, "No memory");
> return -ENOMEM;
> }
> + } else if (request->type == NBD_CMD_BLOCK_STATUS) {
> + request->contexts = &client->contexts;
> }
THere are paths where request->contexts is left NULL but request->type
was set...
> @@ -2841,6 +2848,11 @@ static coroutine_fn void nbd_trip(void *opaque)
> } else {
> ret = nbd_handle_request(client, &request, req->data,
&local_err);
> }
> + if (request.type == NBD_CMD_BLOCK_STATUS &&
> + request.contexts != &client->contexts) {
> + g_free(request.contexts->bitmaps);
> + g_free(request.contexts);
> + }
so I think this also needs to be tweaked to check that
request.contexts is non-NULL before dereferencing to free bitmaps.
agree (and I missed this during my review :)
suggest:
if (request.contexts && request.contexts != &client->contexts) {
g_free(..)
g_free(..)
}
--
Best regards,
Vladimir