* /proc/asound/card0/oss_mixer stack corruption
@ 2014-08-21 17:45 Tommi Rantala
2014-08-21 18:55 ` [alsa-devel] " Clemens Ladisch
0 siblings, 1 reply; 3+ messages in thread
From: Tommi Rantala @ 2014-08-21 17:45 UTC (permalink / raw)
To: Jaroslav Kysela, Takashi Iwai; +Cc: trinity, Dave Jones, LKML, alsa-devel
Hello,
Trinity discovered that writing 128 bytes to
/proc/asound/card0/oss_mixer triggers a stack corruption.
Tommi
# printf %128s > /proc/asound/card0/oss_mixer
ALSA: mixer_oss: invalid OSS volume ''
Kernel panic - not syncing: stack-protector: Kernel stack is corrupted
in: ffffffff81e193ba
CPU: 0 PID: 2778 Comm: bash Not tainted 3.17.0-rc1+ #13
Hardware name: Bochs Bochs, BIOS Bochs 01/01/2011
ffff880039fd4bf0 ffff880034c87bd8 ffffffff8229824a ffffffff828e2a40
ffff880034c87c50 ffffffff8229051d ffff880000000010 ffff880034c87c60
ffff880034c87c00 0000000000000020 ffffffff81e193ba 0000000000000080
Call Trace:
[<ffffffff8229824a>] dump_stack+0x4d/0x66
[<ffffffff8229051d>] panic+0xc8/0x201
[<ffffffff81e193ba>] ? snd_mixer_oss_proc_write+0x24a/0x270
[<ffffffff8113c716>] __stack_chk_fail+0x16/0x20
[<ffffffff81e193ba>] snd_mixer_oss_proc_write+0x24a/0x270
[<ffffffff810a8a27>] ? kvm_clock_read+0x27/0x40
[<ffffffff81dfb54c>] snd_info_entry_release+0x6c/0x110
[<ffffffff812e9af6>] close_pdeo+0x136/0x1a0
[<ffffffff8118bec1>] ? __lock_acquire+0x951/0xb40
[<ffffffff810a8a27>] ? kvm_clock_read+0x27/0x40
[<ffffffff812e9b9e>] proc_reg_release+0x3e/0x60
[<ffffffff8127fa41>] __fput+0x111/0x1e0
[<ffffffff8127fb59>] ____fput+0x9/0x10
[<ffffffff8115e3ee>] task_work_run+0x9e/0xd0
[<ffffffff8106aaa5>] do_notify_resume+0x55/0x70
[<ffffffff822b12e2>] int_signal+0x12/0x17
Kernel Offset: 0x0 from 0xffffffff81000000 (relocation range:
0xffffffff80000000-0xffffffff9fffffff)
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [alsa-devel] /proc/asound/card0/oss_mixer stack corruption
2014-08-21 17:45 /proc/asound/card0/oss_mixer stack corruption Tommi Rantala
@ 2014-08-21 18:55 ` Clemens Ladisch
2014-08-22 5:23 ` Takashi Iwai
0 siblings, 1 reply; 3+ messages in thread
From: Clemens Ladisch @ 2014-08-21 18:55 UTC (permalink / raw)
To: Tommi Rantala, Jaroslav Kysela, Takashi Iwai
Cc: Dave Jones, alsa-devel, LKML, trinity
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:
--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@gmail.com>
Cc: <stable@vger.kernel.org> # v2.2.26+
Signed-off-by: Clemens Ladisch <clemens@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;
}
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [alsa-devel] /proc/asound/card0/oss_mixer stack corruption
2014-08-21 18:55 ` [alsa-devel] " Clemens Ladisch
@ 2014-08-22 5:23 ` Takashi Iwai
0 siblings, 0 replies; 3+ messages in thread
From: Takashi Iwai @ 2014-08-22 5:23 UTC (permalink / raw)
To: Clemens Ladisch
Cc: Tommi Rantala, Jaroslav Kysela, Dave Jones, alsa-devel, LKML,
trinity
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@gmail.com>
> Cc: <stable@vger.kernel.org> # v2.2.26+
> Signed-off-by: Clemens Ladisch <clemens@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;
> }
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2014-08-22 5:23 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-08-21 17:45 /proc/asound/card0/oss_mixer stack corruption Tommi Rantala
2014-08-21 18:55 ` [alsa-devel] " Clemens Ladisch
2014-08-22 5:23 ` Takashi Iwai
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox