On Tue, 27 Feb 2018 18:01:18 +0100,
Richard Fitzgerald wrote:
>
> The patch "ALSA: control: code refactoring for ELEM_READ/ELEM_WRITE
> operations" introduced a potential for kernel memory corruption due
> to an incorrect if statement allowing non-readable controls to fall
> through and call the get function. For TLV controls a driver can omit
> SNDRV_CTL_ELEM_ACCESS_READ to ensure that only the TLV get function
> can be called. Instead the normal get() can be invoked unexpectedly
> and as the driver expects that this will only be called for controls
> <= 512 bytes, potentially try to copy >512 bytes into the 512 byte
> return array, so corrupting kernel memory.
>
> The problem is an attempt to refactor the snd_ctl_elem_read function
> to invert the logic so that it conditionally aborted if the control
> is unreadable instead of conditionally executing. But the if statement
> wasn't inverted correctly.
>
> The correct inversion of
>
> if (a && !b)
>
> is
> if (!a || b)
>
> Fixes: becf9e5d553c2 ("ALSA: control: code refactoring for ELEM_READ/ELEM_WRITE operations")
> Signed-off-by: Richard Fitzgerald <rf@xxxxxxxxxxxxxxxxxxxxx>
> ---
> sound/core/control.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/sound/core/control.c b/sound/core/control.c
> index 0b3026d937b1..8a77620a3854 100644
> --- a/sound/core/control.c
> +++ b/sound/core/control.c
> @@ -889,7 +889,7 @@ static int snd_ctl_elem_read(struct snd_card *card,
>
> index_offset = snd_ctl_get_ioff(kctl, &control->id);
> vd = &kctl->vd[index_offset];
> - if (!(vd->access & SNDRV_CTL_ELEM_ACCESS_READ) && kctl->get == NULL)
> + if (!(vd->access & SNDRV_CTL_ELEM_ACCESS_READ) || kctl->get == NULL)
Doh, it's a common mistake. Thanks for catching this.
It deserves for stable kernel.
Takashi
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
