From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756334Ab0JKU5q (ORCPT ); Mon, 11 Oct 2010 16:57:46 -0400 Received: from cantor.suse.de ([195.135.220.2]:37685 "EHLO mx1.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756241Ab0JKU5p (ORCPT ); Mon, 11 Oct 2010 16:57:45 -0400 Date: Mon, 11 Oct 2010 22:57:40 +0200 Message-ID: From: Takashi Iwai To: Dan Carpenter Cc: Mark Brown , Liam Girdwood , Jaroslav Kysela , Peter Ujfalusi , Jassi Brar , alsa-devel@alsa-project.org, kernel-janitors@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [patch] ASoC: soc: snprintf() doesn't return negative In-Reply-To: <20101011194502.GK5851@bicker> References: <20101011035416.GD5851@bicker> <20101011104009.GB9231@rakim.wolfsonmicro.main> <20101011164038.GE5851@bicker> <20101011185148.GB22355@opensource.wolfsonmicro.com> <20101011194502.GK5851@bicker> User-Agent: Wanderlust/2.15.6 (Almost Unreal) SEMI/1.14.6 (Maruoka) FLIM/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL/10.7 Emacs/23.1 (x86_64-suse-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI 1.14.6 - "Maruoka") Content-Type: text/plain; charset=US-ASCII Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org At Mon, 11 Oct 2010 21:45:02 +0200, Dan Carpenter wrote: > > On Mon, Oct 11, 2010 at 07:51:48PM +0100, Mark Brown wrote: > > In actual fact quite a few devices have enough registers to be > > truncated, meaning that it's not only possible but likely we'll exercise > > the cases that deal with the end of buffer. If snprintf() is returning > > values larger than buffer size it was given we're likely to have an > > issue but it seems that there's something missing in your analysis since > > we're never seeing WARN_ON()s and are instead seeing the behaviour the > > code is intended to give, which is to truncate the output when we run > > out of space. > > > > Could you re-check your analysis, please? > > That's odd. I'm sorry, I can't explain why you wouldn't see a stack > trace... The code is straight forward: > > /* Reject out-of-range values early. Large positive sizes are > used for unknown buffer sizes. */ > if (WARN_ON_ONCE((int) size < 0)) > return 0; > > It would still give you truncated output but after the NULL terminator > there would be information leaked from the kernel. If the reader > program had allocated a large enough buffer to handle the extra > information it wouldn't cause a problem. Well, actually we should fix either: - check the return of snprintf() at each time properly, list_for_each_entry(dai, &dai_list, list) { int len = snprintf(buf + ret, PAGE_SIZE - ret, "%s\n", dai->name); if (len < 0) return len; ret += len; } - or just assume it's never negative (as is on Linux kernel code) In either case, a negative check after for loop is superfluous. And, when no negative return value is assured (or filtered out like above), there can't be overflow, too. snprintf() fills the string at most the size including NUL-char. OTOH, it returns the size that doesn't include NUL-char. Takashi