[kernel] r18864 - in dists/sid/linux-tools/debian: . patches

Ben Hutchings benh at alioth.debian.org
Tue Mar 20 05:03:54 UTC 2012


Author: benh
Date: Tue Mar 20 05:03:49 2012
New Revision: 18864

Log:
Apply upstream changes to fix various buffer overflow bugs

Added:
   dists/sid/linux-tools/debian/patches/perf-tools-incorrect-use-of-snprintf-results-in-segv.patch
   dists/sid/linux-tools/debian/patches/perf-tools-use-scnprintf-where-applicable.patch
Modified:
   dists/sid/linux-tools/debian/changelog
   dists/sid/linux-tools/debian/patches/series

Modified: dists/sid/linux-tools/debian/changelog
==============================================================================
--- dists/sid/linux-tools/debian/changelog	Tue Mar 20 04:32:41 2012	(r18863)
+++ dists/sid/linux-tools/debian/changelog	Tue Mar 20 05:03:49 2012	(r18864)
@@ -1,3 +1,11 @@
+linux-tools (3.2.7-2) unstable; urgency=high
+
+  * Apply upstream changes to fix various buffer overflow bugs:
+    - perf tools: Use scnprintf where applicable
+    - perf tools: Incorrect use of snprintf results in SEGV
+
+ -- Ben Hutchings <ben at decadent.org.uk>  Tue, 20 Mar 2012 04:54:22 +0000
+
 linux-tools (3.2.7-1) unstable; urgency=low
 
   * New upstream stable update:

Added: dists/sid/linux-tools/debian/patches/perf-tools-incorrect-use-of-snprintf-results-in-segv.patch
==============================================================================
--- /dev/null	00:00:00 1970	(empty, because file is newly added)
+++ dists/sid/linux-tools/debian/patches/perf-tools-incorrect-use-of-snprintf-results-in-segv.patch	Tue Mar 20 05:03:49 2012	(r18864)
@@ -0,0 +1,64 @@
+From b832796caa1fda8516464a003c8c7cc547bc20c2 Mon Sep 17 00:00:00 2001
+From: Anton Blanchard <anton at samba.org>
+Date: Wed, 7 Mar 2012 11:42:49 +1100
+Subject: perf tools: Incorrect use of snprintf results in SEGV
+
+From: Anton Blanchard <anton at samba.org>
+
+commit b832796caa1fda8516464a003c8c7cc547bc20c2 upstream.
+
+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.
+
+Cc: David Ahern <dsahern at gmail.com>
+Cc: Eric B Munson <emunson at mgebm.net>
+Cc: Frederic Weisbecker <fweisbec at gmail.com>
+Cc: Ingo Molnar <mingo at elte.hu>
+Cc: Paul Mackerras <paulus at samba.org>
+Cc: Peter Zijlstra <peterz at infradead.org>
+Cc: Yanmin Zhang <yanmin_zhang at linux.intel.com>
+Link: http://lkml.kernel.org/r/20120307114249.44275ca3@kryten
+Signed-off-by: Anton Blanchard <anton at samba.org>
+Signed-off-by: Arnaldo Carvalho de Melo <acme at redhat.com>
+Signed-off-by: Greg Kroah-Hartman <gregkh at linuxfoundation.org>
+
+---
+ tools/perf/util/sort.c |    3 +++
+ 1 file changed, 3 insertions(+)
+
+--- a/tools/perf/util/sort.c
++++ b/tools/perf/util/sort.c
+@@ -33,6 +33,9 @@ static int repsep_snprintf(char *bf, siz
+ 		}
+ 	}
+ 	va_end(ap);
++
++	if (n >= (int)size)
++		return size - 1;
+ 	return n;
+ }
+ 

Added: dists/sid/linux-tools/debian/patches/perf-tools-use-scnprintf-where-applicable.patch
==============================================================================
--- /dev/null	00:00:00 1970	(empty, because file is newly added)
+++ dists/sid/linux-tools/debian/patches/perf-tools-use-scnprintf-where-applicable.patch	Tue Mar 20 05:03:49 2012	(r18864)
@@ -0,0 +1,267 @@
+From e7f01d1e3d8d501deb8abeaa269d5d48a703b8b0 Mon Sep 17 00:00:00 2001
+From: Arnaldo Carvalho de Melo <acme at redhat.com>
+Date: Wed, 14 Mar 2012 12:29:29 -0300
+Subject: perf tools: Use scnprintf where applicable
+
+From: Arnaldo Carvalho de Melo <acme at redhat.com>
+
+commit e7f01d1e3d8d501deb8abeaa269d5d48a703b8b0 upstream.
+
+Several places were expecting that the value returned was the number of
+characters printed, not what would be printed if there was space.
+
+Fix it by using the scnprintf and vscnprintf variants we inherited from
+the kernel sources.
+
+Some corner cases where the number of printed characters were not
+accounted were fixed too.
+
+Reported-by: Anton Blanchard <anton at samba.org>
+Cc: Anton Blanchard <anton at samba.org>
+Cc: Eric B Munson <emunson at mgebm.net>
+Cc: David Ahern <dsahern at gmail.com>
+Cc: Frederic Weisbecker <fweisbec at gmail.com>
+Cc: Mike Galbraith <efault at gmx.de>
+Cc: Paul Mackerras <paulus at samba.org>
+Cc: Peter Zijlstra <peterz at infradead.org>
+Cc: Stephane Eranian <eranian at google.com>
+Cc: Yanmin Zhang <yanmin_zhang at linux.intel.com>
+Link: http://lkml.kernel.org/n/tip-kwxo2eh29cxmd8ilixi2005x@git.kernel.org
+Signed-off-by: Arnaldo Carvalho de Melo <acme at redhat.com>
+Signed-off-by: Greg Kroah-Hartman <gregkh at linuxfoundation.org>
+
+---
+ tools/perf/arch/powerpc/util/header.c |    2 +-
+ tools/perf/arch/x86/util/header.c     |    2 +-
+ tools/perf/util/color.c               |    9 +++++----
+ tools/perf/util/header.c              |    4 ++--
+ tools/perf/util/hist.c                |   30 +++++++++++++++---------------
+ tools/perf/util/strbuf.c              |    7 ++++---
+ tools/perf/util/ui/browsers/hists.c   |   12 ++++++------
+ tools/perf/util/ui/helpline.c         |    2 +-
+ 8 files changed, 35 insertions(+), 33 deletions(-)
+
+--- a/tools/perf/arch/powerpc/util/header.c
++++ b/tools/perf/arch/powerpc/util/header.c
+@@ -25,7 +25,7 @@ get_cpuid(char *buffer, size_t sz)
+ 
+ 	pvr = mfspr(SPRN_PVR);
+ 
+-	nb = snprintf(buffer, sz, "%lu,%lu$", PVR_VER(pvr), PVR_REV(pvr));
++	nb = scnprintf(buffer, sz, "%lu,%lu$", PVR_VER(pvr), PVR_REV(pvr));
+ 
+ 	/* look for end marker to ensure the entire data fit */
+ 	if (strchr(buffer, '$')) {
+--- a/tools/perf/arch/x86/util/header.c
++++ b/tools/perf/arch/x86/util/header.c
+@@ -48,7 +48,7 @@ get_cpuid(char *buffer, size_t sz)
+ 		if (family >= 0x6)
+ 			model += ((a >> 16) & 0xf) << 4;
+ 	}
+-	nb = snprintf(buffer, sz, "%s,%u,%u,%u$", vendor, family, model, step);
++	nb = scnprintf(buffer, sz, "%s,%u,%u,%u$", vendor, family, model, step);
+ 
+ 	/* look for end marker to ensure the entire data fit */
+ 	if (strchr(buffer, '$')) {
+--- a/tools/perf/util/color.c
++++ b/tools/perf/util/color.c
+@@ -1,3 +1,4 @@
++#include <linux/kernel.h>
+ #include "cache.h"
+ #include "color.h"
+ 
+@@ -182,12 +183,12 @@ static int __color_vsnprintf(char *bf, s
+ 	}
+ 
+ 	if (perf_use_color_default && *color)
+-		r += snprintf(bf, size, "%s", color);
+-	r += vsnprintf(bf + r, size - r, fmt, args);
++		r += scnprintf(bf, size, "%s", color);
++	r += vscnprintf(bf + r, size - r, fmt, args);
+ 	if (perf_use_color_default && *color)
+-		r += snprintf(bf + r, size - r, "%s", PERF_COLOR_RESET);
++		r += scnprintf(bf + r, size - r, "%s", PERF_COLOR_RESET);
+ 	if (trail)
+-		r += snprintf(bf + r, size - r, "%s", trail);
++		r += scnprintf(bf + r, size - r, "%s", trail);
+ 	return r;
+ }
+ 
+--- a/tools/perf/util/header.c
++++ b/tools/perf/util/header.c
+@@ -1227,7 +1227,7 @@ int build_id_cache__add_s(const char *sb
+ 	if (realname == NULL || filename == NULL || linkname == NULL)
+ 		goto out_free;
+ 
+-	len = snprintf(filename, size, "%s%s%s",
++	len = scnprintf(filename, size, "%s%s%s",
+ 		       debugdir, is_kallsyms ? "/" : "", realname);
+ 	if (mkdir_p(filename, 0755))
+ 		goto out_free;
+@@ -1242,7 +1242,7 @@ int build_id_cache__add_s(const char *sb
+ 			goto out_free;
+ 	}
+ 
+-	len = snprintf(linkname, size, "%s/.build-id/%.2s",
++	len = scnprintf(linkname, size, "%s/.build-id/%.2s",
+ 		       debugdir, sbuild_id);
+ 
+ 	if (access(linkname, X_OK) && mkdir_p(linkname, 0755))
+--- a/tools/perf/util/hist.c
++++ b/tools/perf/util/hist.c
+@@ -767,7 +767,7 @@ static int hist_entry__pcnt_snprintf(str
+ 						     sep ? "%.2f" : "   %6.2f%%",
+ 						     (period * 100.0) / total);
+ 		else
+-			ret = snprintf(s, size, sep ? "%.2f" : "   %6.2f%%",
++			ret = scnprintf(s, size, sep ? "%.2f" : "   %6.2f%%",
+ 				       (period * 100.0) / total);
+ 		if (symbol_conf.show_cpu_utilization) {
+ 			ret += percent_color_snprintf(s + ret, size - ret,
+@@ -790,20 +790,20 @@ static int hist_entry__pcnt_snprintf(str
+ 			}
+ 		}
+ 	} else
+-		ret = snprintf(s, size, sep ? "%" PRIu64 : "%12" PRIu64 " ", period);
++		ret = scnprintf(s, size, sep ? "%" PRIu64 : "%12" PRIu64 " ", period);
+ 
+ 	if (symbol_conf.show_nr_samples) {
+ 		if (sep)
+-			ret += snprintf(s + ret, size - ret, "%c%" PRIu64, *sep, nr_events);
++			ret += scnprintf(s + ret, size - ret, "%c%" PRIu64, *sep, nr_events);
+ 		else
+-			ret += snprintf(s + ret, size - ret, "%11" PRIu64, nr_events);
++			ret += scnprintf(s + ret, size - ret, "%11" PRIu64, nr_events);
+ 	}
+ 
+ 	if (symbol_conf.show_total_period) {
+ 		if (sep)
+-			ret += snprintf(s + ret, size - ret, "%c%" PRIu64, *sep, period);
++			ret += scnprintf(s + ret, size - ret, "%c%" PRIu64, *sep, period);
+ 		else
+-			ret += snprintf(s + ret, size - ret, " %12" PRIu64, period);
++			ret += scnprintf(s + ret, size - ret, " %12" PRIu64, period);
+ 	}
+ 
+ 	if (pair_hists) {
+@@ -818,25 +818,25 @@ static int hist_entry__pcnt_snprintf(str
+ 		diff = new_percent - old_percent;
+ 
+ 		if (fabs(diff) >= 0.01)
+-			snprintf(bf, sizeof(bf), "%+4.2F%%", diff);
++			ret += scnprintf(bf, sizeof(bf), "%+4.2F%%", diff);
+ 		else
+-			snprintf(bf, sizeof(bf), " ");
++			ret += scnprintf(bf, sizeof(bf), " ");
+ 
+ 		if (sep)
+-			ret += snprintf(s + ret, size - ret, "%c%s", *sep, bf);
++			ret += scnprintf(s + ret, size - ret, "%c%s", *sep, bf);
+ 		else
+-			ret += snprintf(s + ret, size - ret, "%11.11s", bf);
++			ret += scnprintf(s + ret, size - ret, "%11.11s", bf);
+ 
+ 		if (show_displacement) {
+ 			if (displacement)
+-				snprintf(bf, sizeof(bf), "%+4ld", displacement);
++				ret += scnprintf(bf, sizeof(bf), "%+4ld", displacement);
+ 			else
+-				snprintf(bf, sizeof(bf), " ");
++				ret += scnprintf(bf, sizeof(bf), " ");
+ 
+ 			if (sep)
+-				ret += snprintf(s + ret, size - ret, "%c%s", *sep, bf);
++				ret += scnprintf(s + ret, size - ret, "%c%s", *sep, bf);
+ 			else
+-				ret += snprintf(s + ret, size - ret, "%6.6s", bf);
++				ret += scnprintf(s + ret, size - ret, "%6.6s", bf);
+ 		}
+ 	}
+ 
+@@ -854,7 +854,7 @@ int hist_entry__snprintf(struct hist_ent
+ 		if (se->elide)
+ 			continue;
+ 
+-		ret += snprintf(s + ret, size - ret, "%s", sep ?: "  ");
++		ret += scnprintf(s + ret, size - ret, "%s", sep ?: "  ");
+ 		ret += se->se_snprintf(he, s + ret, size - ret,
+ 				       hists__col_len(hists, se->se_width_idx));
+ 	}
+--- a/tools/perf/util/strbuf.c
++++ b/tools/perf/util/strbuf.c
+@@ -1,4 +1,5 @@
+ #include "cache.h"
++#include <linux/kernel.h>
+ 
+ int prefixcmp(const char *str, const char *prefix)
+ {
+@@ -89,14 +90,14 @@ void strbuf_addf(struct strbuf *sb, cons
+ 	if (!strbuf_avail(sb))
+ 		strbuf_grow(sb, 64);
+ 	va_start(ap, fmt);
+-	len = vsnprintf(sb->buf + sb->len, sb->alloc - sb->len, fmt, ap);
++	len = vscnprintf(sb->buf + sb->len, sb->alloc - sb->len, fmt, ap);
+ 	va_end(ap);
+ 	if (len < 0)
+-		die("your vsnprintf is broken");
++		die("your vscnprintf is broken");
+ 	if (len > strbuf_avail(sb)) {
+ 		strbuf_grow(sb, len);
+ 		va_start(ap, fmt);
+-		len = vsnprintf(sb->buf + sb->len, sb->alloc - sb->len, fmt, ap);
++		len = vscnprintf(sb->buf + sb->len, sb->alloc - sb->len, fmt, ap);
+ 		va_end(ap);
+ 		if (len > strbuf_avail(sb)) {
+ 			die("this should not happen, your snprintf is broken");
+--- a/tools/perf/util/ui/browsers/hists.c
++++ b/tools/perf/util/ui/browsers/hists.c
+@@ -839,15 +839,15 @@ static int hists__browser_title(struct h
+ 	unsigned long nr_events = self->stats.nr_events[PERF_RECORD_SAMPLE];
+ 
+ 	nr_events = convert_unit(nr_events, &unit);
+-	printed = snprintf(bf, size, "Events: %lu%c %s", nr_events, unit, ev_name);
++	printed = scnprintf(bf, size, "Events: %lu%c %s", nr_events, unit, ev_name);
+ 
+ 	if (thread)
+-		printed += snprintf(bf + printed, size - printed,
++		printed += scnprintf(bf + printed, size - printed,
+ 				    ", Thread: %s(%d)",
+ 				    (thread->comm_set ? thread->comm : ""),
+ 				    thread->pid);
+ 	if (dso)
+-		printed += snprintf(bf + printed, size - printed,
++		printed += scnprintf(bf + printed, size - printed,
+ 				    ", DSO: %s", dso->short_name);
+ 	return printed;
+ }
+@@ -1097,7 +1097,7 @@ static void perf_evsel_menu__write(struc
+ 						       HE_COLORSET_NORMAL);
+ 
+ 	nr_events = convert_unit(nr_events, &unit);
+-	printed = snprintf(bf, sizeof(bf), "%lu%c%s%s", nr_events,
++	printed = scnprintf(bf, sizeof(bf), "%lu%c%s%s", nr_events,
+ 			   unit, unit == ' ' ? "" : " ", ev_name);
+ 	slsmg_printf("%s", bf);
+ 
+@@ -1107,8 +1107,8 @@ static void perf_evsel_menu__write(struc
+ 		if (!current_entry)
+ 			ui_browser__set_color(browser, HE_COLORSET_TOP);
+ 		nr_events = convert_unit(nr_events, &unit);
+-		snprintf(bf, sizeof(bf), ": %ld%c%schunks LOST!", nr_events,
+-			 unit, unit == ' ' ? "" : " ");
++		printed += scnprintf(bf, sizeof(bf), ": %ld%c%schunks LOST!",
++				     nr_events, unit, unit == ' ' ? "" : " ");
+ 		warn = bf;
+ 	}
+ 
+--- a/tools/perf/util/ui/helpline.c
++++ b/tools/perf/util/ui/helpline.c
+@@ -65,7 +65,7 @@ int ui_helpline__show_help(const char *f
+ 	static int backlog;
+ 
+ 	pthread_mutex_lock(&ui__lock);
+-	ret = vsnprintf(ui_helpline__last_msg + backlog,
++	ret = vscnprintf(ui_helpline__last_msg + backlog,
+ 			sizeof(ui_helpline__last_msg) - backlog, format, ap);
+ 	backlog += ret;
+ 

Modified: dists/sid/linux-tools/debian/patches/series
==============================================================================
--- dists/sid/linux-tools/debian/patches/series	Tue Mar 20 04:32:41 2012	(r18863)
+++ dists/sid/linux-tools/debian/patches/series	Tue Mar 20 05:03:49 2012	(r18864)
@@ -1,3 +1,5 @@
+perf-tools-incorrect-use-of-snprintf-results-in-segv.patch
+perf-tools-use-scnprintf-where-applicable.patch
 modpost-symbol-prefix.patch
 tools-perf-version.patch
 tools-perf-install.patch



More information about the Kernel-svn-changes mailing list