[PATCH] ASoC: SOF: ipc3-topology: Correct get_control_data for non bytes payload

[Péter Ujfalusi]

On 27/04/2022 13:36, Sergey Senozhatsky wrote:
> On (22/04/27 11:50), Peter Ujfalusi wrote:
>> @@ -784,16 +785,23 @@ static int sof_get_control_data(struct snd_soc_component *scomp,
>>  		}
>>  
>>  		cdata = wdata[i].control->ipc_control_data;
>> -		wdata[i].pdata = cdata->data;
>> -		if (!wdata[i].pdata)
>> -			return -EINVAL;
>>  
>>  		/* make sure data is valid - data can be updated at runtime */
>> -		if (widget->dobj.widget.kcontrol_type[i] == SND_SOC_TPLG_TYPE_BYTES &&
>> -		    wdata[i].pdata->magic != SOF_ABI_MAGIC)
>> -			return -EINVAL;
>> +		if (widget->dobj.widget.kcontrol_type[i] == SND_SOC_TPLG_TYPE_BYTES) {
>> +			if (!cdata->data)
>> +				return -EINVAL;
>> +
>> +			if (cdata->data->magic != SOF_ABI_MAGIC)
>> +				return -EINVAL;
>> +
>> +			wdata[i].pdata = cdata->data->data;
>> +			wdata[i].pdata_size = cdata->data->size;
>> +		} else {
>> +			wdata[i].pdata = cdata->chanv; /* points to the control data union */
>> +			wdata[i].pdata_size = wdata[i].control->size;
> 				^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> A question, so here wdata[i].control->size is
> 
> 	scontrol->size = struct_size(scontrol->control_data, chanv,
> 					le32_to_cpu(mc->num_channels));
> 
> Right?
> 
> If so, then is this really what we memcpy()? We memcpy() control->data->chanv
> and its size is `sizeof(chanv) * le32_to_cpu(mc->num_channels)`, isn't it?
> 
> [..]
>>  	if (ipc_data_size) {
>>  		for (i = 0; i < widget->num_kcontrols; i++) {
>> +			if (!wdata[i].pdata_size)
>> +				continue;
>> +
>> +			memcpy(&process->data[offset], wdata[i].pdata,
>> +			       wdata[i].pdata_size);
>> +			offset += wdata[i].pdata_size;
>>  		}
>>  	}
> 
> So should sof_get_control_data() have instead of this
> 
> 	wdata[i].pdata_size = wdata[i].control->size;
> 
> something like this
> 
> 	wdata[i].pdata_size = wdata[i].control->size - sizeof(struct sof_ipc_ctrl_data);

Yes, you are right.

> 
> So that we will copy payload data, which is a bunch of chanv structs 8
> bytes each.

-- 
Péter