From patchwork Thu Aug 21 18:55:21 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Clemens Ladisch X-Patchwork-Id: 4759501 X-Patchwork-Delegate: tiwai@suse.de Return-Path: X-Original-To: patchwork-alsa-devel@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork2.web.kernel.org (Postfix) with ESMTP id C95ECC0338 for ; Thu, 21 Aug 2014 18:57:04 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id EBD102018E for ; Thu, 21 Aug 2014 18:57:03 +0000 (UTC) Received: from alsa0.perex.cz (alsa0.perex.cz [77.48.224.243]) by mail.kernel.org (Postfix) with ESMTP id 9B3B1200DB for ; Thu, 21 Aug 2014 18:57:02 +0000 (UTC) Received: by alsa0.perex.cz (Postfix, from userid 1000) id 6D4FE2658DC; Thu, 21 Aug 2014 20:57:00 +0200 (CEST) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Spam-Level: X-Spam-Status: No, score=-1.9 required=5.0 tests=BAYES_00, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 Received: from alsa0.perex.cz (localhost [IPv6:::1]) by alsa0.perex.cz (Postfix) with ESMTP id 91DE52658DB; Thu, 21 Aug 2014 20:56:49 +0200 (CEST) X-Original-To: alsa-devel@alsa-project.org Delivered-To: alsa-devel@alsa-project.org Received: by alsa0.perex.cz (Postfix, from userid 1000) id 4B5822658E5; Thu, 21 Aug 2014 20:56:48 +0200 (CEST) Received: from dehamd003.servertools24.de (dehamd003.servertools24.de [31.47.254.18]) by alsa0.perex.cz (Postfix) with ESMTP id 2C5FF2658DB for ; Thu, 21 Aug 2014 20:56:40 +0200 (CEST) Received: from [192.168.42.201] (tmo-106-89.customers.d1-online.com [80.187.106.89]) by dehamd003.servertools24.de (Postfix) with ESMTPSA id 897BFF8063; Thu, 21 Aug 2014 20:56:23 +0200 (CEST) Message-ID: <53F64099.8000601@ladisch.de> Date: Thu, 21 Aug 2014 20:55:21 +0200 From: Clemens Ladisch User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.0 MIME-Version: 1.0 To: Tommi Rantala , Jaroslav Kysela , Takashi Iwai References: In-Reply-To: X-PPP-Message-ID: <20140821185625.936109.29856@dehamd003.servertools24.de> X-PPP-Vhost: ladisch.de Cc: Dave Jones , alsa-devel@alsa-project.org, LKML , trinity@vger.kernel.org Subject: Re: [alsa-devel] /proc/asound/card0/oss_mixer stack corruption X-BeenThere: alsa-devel@alsa-project.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: "Alsa-devel mailing list for ALSA developers - http://www.alsa-project.org" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: alsa-devel-bounces@alsa-project.org Sender: alsa-devel-bounces@alsa-project.org X-Virus-Scanned: ClamAV using ClamSMTP Tommi Rantala wrote: > Trinity discovered that writing 128 bytes to > /proc/asound/card0/oss_mixer triggers a stack corruption. > > Call Trace: > [] __stack_chk_fail+0x16/0x20 > [] 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 Cc: # v2.2.26+ Signed-off-by: Clemens Ladisch --- 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; }