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

[Takashi Iwai <tiwai@suse.de>]

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.


Takashi