Skip to contentSkip to conversation

tracing: A couple of fixes to tracepoints

GIT PULL New Steven Rostedt 1 participant 3 messages May 8, 2014

Subsystems (2)
lore
Share & tools
lore

Steven Rostedt

author maintainer
#0 lore raw

Linus,

This contains two fixes.

The first is a long standing bug that causes bogus data to show up in the refcnt field of the module_refcnt tracepoint. It was introduced by a merge conflict resolution back in 2.6.35-rc days.

The result should be refcnt = incs - decs, but instead it did refcnt = incs + decs.

The second fix is to a bug that was introduced in this merge window that allowed for a tracepoint funcs pointer to be used after it was freed. Moving the location of where the probes are released solved the problem.

Please pull the latest trace-fixes-v3.15-rc4-v2 tree, which can be found at:

git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-trace.git
trace-fixes-v3.15-rc4-v2

Tag SHA1: 3c46db2195fb760afba9f01a495557e78ca8226e Head SHA1: 8058bd0faad860e75547cc5cb5d4ade016247a79

Mathieu Desnoyers (1):
tracepoint: Fix use of tracepoint funcs after rcu free

Romain Izard (1):
trace: module: Maintain a valid user count

----

 include/trace/events/module.h | 2 +-
 kernel/tracepoint.c           | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)
author maintainer
#1 lore raw

PATCH 1/2 trace: module: Maintain a valid user count

From: Romain Izard <[email protected]>

The replacement of the 'count' variable by two variables 'incs' and 'decs' to resolve some race conditions during module unloading was done in parallel with some cleanup in the trace subsystem, and was integrated as a merge.

Unfortunately, the formula for this replacement was wrong in the tracing code, and the refcount in the traces was not usable as a result.

Use 'count = incs - decs' to compute the user count.

Cc Rusty Russell <[email protected]>
Cc Frederic Weisbecker <[email protected]>
Cc [email protected] # 2.6.35
Fixes c1ab9cab7509 "merge conflict resolution"
---
 include/trace/events/module.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
include/trace/events/module.h +1 -1
index 11fd51b..daa60c7 100644
@@ -80,7 +80,7 @@ DECLARE_EVENT_CLASS(module_refcnt,
 
 	TP_fast_assign(
 		__entry->ip	= ip;
-		__entry->refcnt	= __this_cpu_read(mod->refptr->incs) + __this_cpu_read(mod->refptr->decs);
+		__entry->refcnt	= __this_cpu_read(mod->refptr->incs) - __this_cpu_read(mod->refptr->decs);
 		__assign_str(name, mod->name);
 	),
 
Signature · 1 line
1.8.5.3
author maintainer
#2 lore raw

PATCH 2/2 tracepoint: Fix use of tracepoint funcs after rcu free

In reply to Steven Rostedt

From: Mathieu Desnoyers <[email protected]>

Commit de7b2973903c "tracepoint: Use struct pointer instead of name hash for reg/unreg tracepoints" introduces a use after free by calling release_probes on the old struct tracepoint array before the newly allocated array is published with rcu_assign_pointer. There is a race window where tracepoints (RCU readers) can perform a "use-after-grace-period-after-free", which shows up as a GPF in stress-tests.

CC Oleg Nesterov <[email protected]>
CC Dave Jones <[email protected]>
Fixes de7b2973903c "tracepoint: Use struct pointer instead of name hash for reg/unreg tracepoints"
---
 kernel/tracepoint.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
kernel/tracepoint.c +2 -2
index ac5b23c..6620e58 100644
@@ -188,7 +188,6 @@ static int tracepoint_add_func(struct tracepoint *tp,
 		WARN_ON_ONCE(1);
 		return PTR_ERR(old);
 	}
-	release_probes(old);
 
 	/*
 	 * rcu_assign_pointer has a smp_wmb() which makes sure that the new
@@ -200,6 +199,7 @@ static int tracepoint_add_func(struct tracepoint *tp,
 	rcu_assign_pointer(tp->funcs, tp_funcs);
 	if (!static_key_enabled(&tp->key))
 		static_key_slow_inc(&tp->key);
+	release_probes(old);
 	return 0;
 }
 
@@ -221,7 +221,6 @@ static int tracepoint_remove_func(struct tracepoint *tp,
 		WARN_ON_ONCE(1);
 		return PTR_ERR(old);
 	}
-	release_probes(old);
 
 	if (!tp_funcs) {
 		/* Removed last function */
@@ -232,6 +231,7 @@ static int tracepoint_remove_func(struct tracepoint *tp,
 			static_key_slow_dec(&tp->key);
 	}
 	rcu_assign_pointer(tp->funcs, tp_funcs);
+	release_probes(old);
 	return 0;
 }
 
Signature · 1 line
1.8.5.3