[alsa-devel] /proc/asound/card0/oss_mixer stack corruption
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)
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; }
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;
participants (3)
-
Clemens Ladisch -
Takashi Iwai -
Tommi Rantala