[Libguestfs] [PATCH v4 23/24] nbd/server: Prepare for per-request filtering of BLOCK_STATUS

[Vladimir Sementsov-Ogievskiy]

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 at 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