linux-kernel - Re: [PATCH] ALSA: control: Make sure that id->index does not overflow in function snd_ctl

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [day] [month] [year] [list]

Message-ID: <s5hzhx1zmxu.wl-tiwai@suse.de>
Date:   Sat, 01 Sep 2018 09:39:09 +0200
From:   Takashi Iwai <tiwai@...e.de>
To:     "Xiao Yang" <YangX92@...mail.com>
Cc:     "alsa-devel@...a-project.org" <alsa-devel@...a-project.org>,
        "92siuyang@...il.com" <92siuyang@...il.com>,
        "rf@...nsource.cirrus.com" <rf@...nsource.cirrus.com>,
        "o-takashi@...amocchi.jp" <o-takashi@...amocchi.jp>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "viro@...iv.linux.org.uk" <viro@...iv.linux.org.uk>
Subject: Re: [PATCH]  ALSA: control: Make sure that id->index does not overflow in function  snd_ctl_replace

On Fri, 31 Aug 2018 13:00:02 +0200,
Xiao Yang wrote:
> 
> From: Young_X <YangX92@...mail.com>
> 
>     The ALSA control code expects that the range of assigned indices to a 
>     control is continuous and does not overflow. Currently there are no 
>     checks to enforce this.
>     If a control with a overflowing index range is created that control 
>     becomes effectively inaccessible and unremovable since 
>     snd_ctl_find_id() will not be able to find it. This patch adds a check 
>     that makes sure that controls with a overflowing index range can not 
>     be created.
>     (same issue as CVE-2014-4656)
> 
> Signed-off-by: Young_X <YangX92@...mail.com>

Thanks for the patch.

Actually this function has been never used (at least for years), hence
the best "fix" would be just to get rid of the whole function, IMO.


Takashi

> ---
>  sound/core/control.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/sound/core/control.c b/sound/core/control.c
> index 9aa15bf..6435772 100644
> --- a/sound/core/control.c
> +++ b/sound/core/control.c
> @@ -441,6 +441,11 @@ int snd_ctl_replace(struct snd_card *card, struct snd_kcontrol *kcontrol,
>  		goto error;
>  	}
>  	id = kcontrol->id;
> +	if (id.index > UINT_MAX - kcontrol->count) {
> +		ret = -EINVAL;
> +		goto error;
> +	}
> +
>  	down_write(&card->controls_rwsem);
>  	old = snd_ctl_find_id(card, &id);
>  	if (!old) {
> -- 
> 2.7.4
> 
>