* /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