Re: [PATCH] ALSA: control: Verify put() result when in debug mode

[Cezary Rojewski <cezary.rojewski@intel.com>]

On 2026-01-30 4:14 PM, Takashi Iwai wrote:
> On Fri, 30 Jan 2026 16:00:10 +0100,
> Cezary Rojewski wrote:
>>
>> On 2026-01-30 3:29 PM, Takashi Iwai wrote:
>>> On Fri, 30 Jan 2026 15:12:58 +0100,
>>> Mark Brown wrote:
>>>>> +	struct snd_ctl_elem_value original;
>>>>
>>>>> +	ret = kctl->get(kctl, &original);
>>>>> +	if (ret)
>>>>> +		return ret;
>>>>
>>>>> +	retcmp = memcmp(&original.value.bytes.data[0], &control->value.bytes.data[0],
>>>>> +			sizeof(original.value.bytes.data[0]));
>>>>> +	if (retcmp)
>>>>> +		retcmp = 1;
>>>>
>>>> original was just allocated from the stack so who knows what values it
>>>> had originally, and the get() is only going to write the part of the
>>>> value that has data since the normal get() path has a memset() in the
>>>> core.  Similarly with the new value coming in from userspace there's no
>>>> requirement for userspace to set anything that isn't part of the value
>>>> being written to any particular value.  This means we're liable to get
>>>> spurious mismatches.
>>>
>>> Yes, and if I understand correctly, the above memcmpy() just compare
>>> the single byte from original and the result?  Then it'll lead to
>>> false-positive outputs.
>>>
>>> We'll need to query the control info and check the relevant values
>>> for each info->type and count.
>>
>> Wouldn't memset(0) as a preparation-step solve the issue? That is,
>> each element - instance of struct snd_ctl_elem_value - has up to 512
>> bytes of data, regardless of type. Not seeing usefulness of ->info()
>> here. Perhaps I'm missing something.
> 
> Well, you're calling memcmp with sizeof(original.value.bytes.data[0]),
> and this size is just one byte, no?
> 
> And, if you need to compare the full data, the remaining space has to
> be initialized, yes.  For the variable original, you can do memset(),
> but for control->value, there is no guarantee of initialization of
> remaining space because it's a copy from user-space.

I admit, I'm a bit embarrassed by the 
"sizeof(original.value.bytes.data[0])" mistake of mine, fixed in v2.

The ->info() operation brings questions to my mind but I believe in v2 
I've utilized it as suggested: use info->type and info->count to 
sanitize the 'new' value. If I'm wrong about that, let me know in the v2 
review.

Kind regards,
Czarek