Re: [PATCH] perf: Incorrect use of snprintf results in SEGV

Arnaldo Carvalho de Melo
2012-03-07T01:09:22Z

Em Wed, Mar 07, 2012 at 11:42:49AM +1100, Anton Blanchard escreveu:
>
> I have a workload where perf top scribbles over the stack and we
> SEGV. What makes it interesting is that an snprintf is causing this.
>
> The workload is a c++ gem that has method names over 3000 characters
> long, but snprintf is designed to avoid overrunning buffers. So what
> went wrong?
>
> The problem is we assume snprintf returns the number of characters
> written:
>
> ret += repsep_snprintf(bf + ret, size - ret, "[%c] ", self->level);
> ...
> ret += repsep_snprintf(bf + ret, size - ret, "%s", self->ms.sym->name);
>
> Unfortunately this is not how snprintf works. snprintf returns the
> number of characters that would have been written if there was enough
> space. In the above case, if the first snprintf returns a value larger
> than size, we pass a negative size into the second snprintf and
> happily scribble over the stack. If you have 3000 character c++
> methods thats a lot of stack to trample.
>
> This patch fixes repsep_snprintf by clamping the value at size - 1
> which is the maximum snprintf can write before adding the NULL
> terminator.
>
> I get the sinking feeling that there are a lot of other uses of
> snprintf that have this same bug, we should audit them all.

Indeed, I wonder what kind of crack pipe I was smoking when I read the
snprintf man page.

Or what kind of such pipe the people who designed snprintf were using
:-(

>
> Signed-off-by: Anton Blanchard <anton@samba.org>
> Cc: stable@kernel.org
> ---
>
> Index: linux-build/tools/perf/util/sort.c
> ===================================================================
> --- linux-build.orig/tools/perf/util/sort.c 2012-03-07 10:58:57.502318907 +1100
> +++ linux-build/tools/perf/util/sort.c 2012-03-07 11:00:58.812423271 +1100
> @@ -33,6 +33,9 @@ static int repsep_snprintf(char *bf, siz
> }
> }
> va_end(ap);
> +
> + if (n >= (int)size)
> + return size - 1;
> return n;
> }
>