linux-kernel - Re: [alsa-devel] /proc/asound/card0/oss

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>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <s5h38cpumgo.wl-tiwai@suse.de>
Date:	Fri, 22 Aug 2014 07:23:19 +0200
From:	Takashi Iwai <tiwai@...e.de>
To:	Clemens Ladisch <clemens@...isch.de>
Cc:	Tommi Rantala <tt.rantala@...il.com>,
	Jaroslav Kysela <perex@...ex.cz>,
	Dave Jones <davej@...hat.com>, alsa-devel@...a-project.org,
	LKML <linux-kernel@...r.kernel.org>, trinity@...r.kernel.org
Subject: Re: [alsa-devel] /proc/asound/card0/oss_mixer stack corruption

At Thu, 21 Aug 2014 20:55:21 +0200,
Clemens Ladisch wrote:
> 
> Tommi Rantala wrote:
> > Trinity discovered that writing 128 bytes to
> > /proc/asound/card0/oss_mixer triggers a stack corruption.
> >
> > Call Trace:
> >  [<ffffffff8113c716>] __stack_chk_fail+0x16/0x20
> >  [<ffffffff81e193ba>] snd_mixer_oss_proc_write+0x24a/0x270
> 
> snd_info_get_line() wants the len parameter to be one less than the
> buffer size, but it isn't:
> 
> 	while (!snd_info_get_line(buffer, line, sizeof(line))) {
> 
> Not that *any* other caller got it correct either:
> 
> sound/core/oss/pcm_oss.c:	while (!snd_info_get_line(buffer, line, sizeof(line))) {
> sound/core/pcm.c:	if (!snd_info_get_line(buffer, line, sizeof(line)))
> sound/core/pcm_memory.c:	if (!snd_info_get_line(buffer, line, sizeof(line))) {
> sound/drivers/dummy.c:	while (!snd_info_get_line(buffer, line, sizeof(line))) {
> sound/pci/ac97/ac97_proc.c:	while (!snd_info_get_line(buffer, line, sizeof(line))) {
> sound/pci/ca0106/ca0106_proc.c:        while (!snd_info_get_line(buffer, line, sizeof(line))) {
> sound/pci/ca0106/ca0106_proc.c:        while (!snd_info_get_line(buffer, line, sizeof(line))) {
> sound/pci/ca0106/ca0106_proc.c:        while (!snd_info_get_line(buffer, line, sizeof(line))) {
> sound/pci/emu10k1/emu10k1x.c:	while (!snd_info_get_line(buffer, line, sizeof(line))) {
> sound/pci/emu10k1/emuproc.c:	while (!snd_info_get_line(buffer, line, sizeof(line))) {
> sound/pci/emu10k1/emuproc.c:	while (!snd_info_get_line(buffer, line, sizeof(line))) {
> sound/pci/hda/hda_eld.c:	while (!snd_info_get_line(buffer, line, sizeof(line))) {
> sound/pci/ice1712/pontis.c:	while (!snd_info_get_line(buffer, line, sizeof(line))) {
> sound/pci/ice1712/prodigy_hifi.c:	while (!snd_info_get_line(buffer, line, sizeof(line))) {
> sound/pci/lola/lola_proc.c:	while (!snd_info_get_line(buffer, line, sizeof(line))) {
> sound/pci/pcxhr/pcxhr.c:	while (!snd_info_get_line(buffer, line, sizeof(line))) {
> 
> Oh well.  At least these proc files are writable only by root, and the
> fix is easy:

Indeed.  Applied now, thanks.


Takashi

> 
> --8<---------------------------------------------------------------->8--
> ALSA: core: fix buffer overflow in snd_info_get_line()
> 
> snd_info_get_line() documents that its last parameter must be one
> less than the buffer size, but this API design guarantees that
> (literally) every caller gets it wrong.
> 
> Just change this parameter to have its obvious meaning.
> 
> Reported-by: Tommi Rantala <tt.rantala@...il.com>
> Cc: <stable@...r.kernel.org> # v2.2.26+
> Signed-off-by: Clemens Ladisch <clemens@...isch.de>
> ---
>  sound/core/info.c |    4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> --- a/sound/core/info.c
> +++ b/sound/core/info.c
> @@ -684,7 +684,7 @@ int snd_info_card_free(struct snd_card *card)
>   * snd_info_get_line - read one line from the procfs buffer
>   * @buffer: the procfs buffer
>   * @line: the buffer to store
> - * @len: the max. buffer size - 1
> + * @len: the max. buffer size
>   *
>   * Reads one line from the buffer and stores the string.
>   *
> @@ -704,7 +704,7 @@ int snd_info_get_line(struct snd_info_buffer *buffer, char *line, int len)
>  			buffer->stop = 1;
>  		if (c == '\n')
>  			break;
> -		if (len) {
> +		if (len > 1) {
>  			len--;
>  			*line++ = c;
>  		}
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/