Hi Denis,


On 28/03/18 21:53, Denis Kenzior wrote:
> Hi Jonas,
>
>> @@ -2308,17 +2304,18 @@ uint16_t qmi_service_send(struct qmi_service 
>> *service,
>>       if (!data)
>>           return 0;
>>   -    data->service = service;
>> -    data->param = param;
>>       data->func = func;
>>       data->user_data = user_data;
>>       data->destroy = destroy;
>>         req = __request_alloc(service->type, service->client_id,
>>                   message, QMI_SERVICE_HDR_SIZE,
>> -                data->param ? data->param->data : NULL,
>> -                data->param ? data->param->length : 0,
>> +                param ? param->data : NULL,
>> +                param ? param->length : 0,
>>                   service_send_callback, data, (void **) &hdr);
>> +
>> +    qmi_param_free(param);
>> +
>
> This looks dangerous.  The problem is that all the qmi code issues 
> qmi_param_free in case qmi_service_send failed.  So you have a double 
> free situation here.
>
>>       if (!req) {
>>           g_free(data);
>>           return 0;
>>
>
So, I checked this again.  The above, as you pointed out, is not right.  
But just moving the qmi_param_free to the end of the qmi_service_send 
function, just before it returns success, is what this should have 
been.  If the function returns success, the user expects that we free 
param; if we do it there then we don't need the reference into the callback.

I'll fix the patch and resubmit it.

/Jonas

> Regards,
> -Denis