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

Date:	Thu, 21 Aug 2014 20:55:21 +0200
From:	Clemens Ladisch <clemens@...isch.de>
To:	Tommi Rantala <tt.rantala@...il.com>,
	Jaroslav Kysela <perex@...ex.cz>, Takashi Iwai <tiwai@...e.de>
CC:	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

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@...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/