Commit 547305a6 authored by Steven Rostedt (VMware)'s avatar Steven Rostedt (VMware) Committed by Peter Zijlstra

tracepoint: Fix out of sync data passing by static caller

Naresh reported a bug that appears to be a side effect of the static
calls. It happens when going from more than one tracepoint callback to
a single one, and removing the first callback on the list. The list of
tracepoint callbacks holds data and a function to call with the
parameters of that tracepoint and a handler to the associated data.

 old_list:
	0: func = foo; data = NULL;
	1: func = bar; data = &bar_struct;

 new_list:
	0: func = bar; data = &bar_struct;

	CPU 0				CPU 1
	-----				-----
   tp_funcs = old_list;
   tp_static_caller = tp_interator

   __DO_TRACE()

    data = tp_funcs[0].data = NULL;

				   tp_funcs = new_list;
				   tracepoint_update_call()
				      tp_static_caller = tp_funcs[0] = bar;
    tp_static_caller(data)
       bar(data)
         x = data->item = NULL->item

       BOOM!

To solve this, add a tracepoint_synchronize_unregister() between
changing tp_funcs and updating the static tracepoint, that does both a
synchronize_rcu() and synchronize_srcu(). This will ensure that when
the static call is updated to the single callback that it will be
receiving the data that it registered with.

Fixes: d25e37d8 ("tracepoint: Optimize using static_call()")
Reported-by: default avatarNaresh Kamboju <naresh.kamboju@linaro.org>
Signed-off-by: default avatarSteven Rostedt (VMware) <rostedt@goodmis.org>
Signed-off-by: default avatarPeter Zijlstra (Intel) <peterz@infradead.org>
Link: https://lore.kernel.org/linux-next/CA+G9fYvPXVRO0NV7yL=FxCmFEMYkCwdz7R=9W+_votpT824YJA@mail.gmail.com
parent de394e75
......@@ -221,7 +221,7 @@ static void *func_remove(struct tracepoint_func **funcs,
return old;
}
static void tracepoint_update_call(struct tracepoint *tp, struct tracepoint_func *tp_funcs)
static void tracepoint_update_call(struct tracepoint *tp, struct tracepoint_func *tp_funcs, bool sync)
{
void *func = tp->iterator;
......@@ -229,8 +229,17 @@ static void tracepoint_update_call(struct tracepoint *tp, struct tracepoint_func
if (!tp->static_call_key)
return;
if (!tp_funcs[1].func)
if (!tp_funcs[1].func) {
func = tp_funcs[0].func;
/*
* If going from the iterator back to a single caller,
* we need to synchronize with __DO_TRACE to make sure
* that the data passed to the callback is the one that
* belongs to that callback.
*/
if (sync)
tracepoint_synchronize_unregister();
}
__static_call_update(tp->static_call_key, tp->static_call_tramp, func);
}
......@@ -265,7 +274,7 @@ static int tracepoint_add_func(struct tracepoint *tp,
* include/linux/tracepoint.h using rcu_dereference_sched().
*/
rcu_assign_pointer(tp->funcs, tp_funcs);
tracepoint_update_call(tp, tp_funcs);
tracepoint_update_call(tp, tp_funcs, false);
static_key_enable(&tp->key);
release_probes(old);
......@@ -297,11 +306,12 @@ static int tracepoint_remove_func(struct tracepoint *tp,
tp->unregfunc();
static_key_disable(&tp->key);
rcu_assign_pointer(tp->funcs, tp_funcs);
} else {
tracepoint_update_call(tp, tp_funcs);
}
rcu_assign_pointer(tp->funcs, tp_funcs);
tracepoint_update_call(tp, tp_funcs,
tp_funcs[0].func != old[0].func);
}
release_probes(old);
return 0;
}
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment