public inbox for kernel-janitors@vger.kernel.org
 help / color / mirror / Atom feed
From: Mark Brown <broonie@opensource.wolfsonmicro.com>
To: Takashi Iwai <tiwai@suse.de>
Cc: alsa-devel@alsa-project.org, Dan Carpenter <error27@gmail.com>,
	Jassi Brar <jassi.brar@samsung.com>,
	kernel-janitors@vger.kernel.org,
	Peter Ujfalusi <peter.ujfalusi@nokia.com>,
	linux-kernel@vger.kernel.org, Liam Girdwood <lrg@slimlogic.co.uk>
Subject: Re: [patch] ASoC: soc: snprintf() doesn't return negative
Date: Tue, 12 Oct 2010 09:35:07 +0000	[thread overview]
Message-ID: <20101012093506.GB30933@rakim.wolfsonmicro.main> (raw)
In-Reply-To: <s5hpqvg1m7v.wl%tiwai@suse.de>

On Mon, Oct 11, 2010 at 10:57:40PM +0200, Takashi Iwai wrote:
> Dan Carpenter wrote:

> 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;
> 	}

In this case we're deliberately eating the error since all these files
are about getting diagnostics out - the code is intentionally soldiering
on and trying to get as much data out as possible rather than giving up
on error.

> In either case, a negative check after for loop is superfluous.

In those ones, yes - it's pretty much there for paranoia since the copy
to userspace is more likely to explode than random memory corruption.

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

Dan was saying that it would return sizes larger than the string it
wrote (which is a behaviour of some implementations) which would be
an issue since it would cause us to pass bad buffer pointers into
subsequent snprintf() calls.

I've not had time to look at this properly but Dan's analysis seems off.

  reply	other threads:[~2010-10-12  9:35 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-10-11  3:54 [patch] ASoC: soc: snprintf() doesn't return negative Dan Carpenter
2010-10-11 10:07 ` Liam Girdwood
2010-10-11 10:40 ` Mark Brown
2010-10-11 16:40   ` Dan Carpenter
2010-10-11 18:51     ` Mark Brown
2010-10-11 19:45       ` Dan Carpenter
2010-10-11 20:57         ` Takashi Iwai
2010-10-12  9:35           ` Mark Brown [this message]
2010-10-12  9:49             ` Takashi Iwai
2010-10-12  9:56               ` Mark Brown
2010-10-12 10:40                 ` Dan Carpenter
2010-10-11 21:11         ` Dan Carpenter

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20101012093506.GB30933@rakim.wolfsonmicro.main \
    --to=broonie@opensource.wolfsonmicro.com \
    --cc=alsa-devel@alsa-project.org \
    --cc=error27@gmail.com \
    --cc=jassi.brar@samsung.com \
    --cc=kernel-janitors@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lrg@slimlogic.co.uk \
    --cc=peter.ujfalusi@nokia.com \
    --cc=tiwai@suse.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox