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

[Arnaldo Carvalho de Melo <acme@redhat.com>]

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