Fwd: Re: [alsa-devel] /proc/asound/card0/oss_mixer stack corruption

[David Henningsson]

Just cross-posting this here because it looks quite severe. (And 
possibly a security issue?)

We don't have any oss_mixer files AFAIK, but according to the post 
below, it should apply to eld* files too, present for almost every HDMI 
audio card out there.
So I tried "printf %64s > /proc/asound/card0/eld#0.0" but did not notice 
anything - but maybe this is either because of the stack layout of the 
actual function, or because we configure the kernel without the stack 
checking used here to discover.

Either way, looks like it should be fixed ASAP.

-------- Forwarded Message --------
Subject: Re: [alsa-devel] /proc/asound/card0/oss_mixer stack corruption
Date: Fri, 22 Aug 2014 07:23:19 +0200
From: Takashi Iwai <tiwai at suse.de>
To: Clemens Ladisch <clemens at ladisch.de>
CC: Tommi Rantala <tt.rantala at gmail.com>, alsa-devel at alsa-project.org, 
LKML <linux-kernel at vger.kernel.org>, trinity at vger.kernel.org, Dave Jones 
<davej at redhat.com>

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 at gmail.com>
> Cc: <stable at vger.kernel.org> # v2.2.26+
> Signed-off-by: Clemens Ladisch <clemens at ladisch.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;
>  		}
>
_______________________________________________
Alsa-devel mailing list
Alsa-devel at alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel