Re: [PATCH 2/3] ALSA: control: add dimension validator for userspace element

[Takashi Sakamoto <o-takashi@sakamocchi.jp>]

On Jul 6 2016 22:34, Takashi Iwai wrote:
> Yes, a validation of info->count is a good idea.
> And it can be even a simple WARN_ON().  It's a clear driver bug and
> it's better to be exposed as loudly as possible.

OK.

Then another question. The same function, snd_ctl_elem_info() uses a
combination of  at CONFIG_SND_DEBUG and snd_BUG_ON() to detect
info->access bug of each driver.

http://git.kernel.org/cgit/linux/kernel/git/tiwai/sound.git/tree/sound/core/control.c#n823

I guess that this is an assertion that each driver must not touch
info->access in its implementations of 'snd_kcontrol_info_t'. On the
other hand, the detection is just enabled at CONFIG_SND_DEBUG.

To me, it's strange, because ALSA control core changes its behaviour
depending on CONFIG_SND_DEBUG. If the option is off, buggy driver works.
Else, it brings kernel panic. This is quite confusing to both developers
of each driver and userspace applications.

In ALSA kernel/userspace interfaces, info->access is described as read-only.
http://git.kernel.org/cgit/linux/kernel/git/tiwai/sound.git/tree/include/uapi/sound/asound.h#n886

Therefore, I think it better to set zero in advance of calling
'snd_kcontrol_info_t' then check the return value with WARN_ON macro,
regardless of CONFIG_SND_DEBUG.

Or should I apply snd_WARN_ON() or snd_BUG_ON() to validators of
info->type and info->count to keep code consistency?


Regards

Takashi Sakamoto