tracing: hist trigger KASAN fixes

Dmitry Vyukov found and reported an issue with hist triggers when running the hist trigger selftests, which Steve Rostedt sent a patch for and which fixed part of the problem; I copied his patch to fix another similar problem in the same code. The result is the first patch in this series.

After that fix was applied, another problem appeared, again triggered by the selftests. The second patch here fixes that.

I then ran my exhaustive testsuite with KASAN enabled and didn't find anything else beyond those.

The following changes since commit 02184c60eba8491ea574cd17b8ba766c86d468f2:

Merge tag 'for-v4.7-rc' of git://git.kernel.org/pub/scm/linux/kernel/git/sre/linux-power-supply (2016-06-27 20:43:00 -0700)

are available in the git repository at:

git://git.yoctoproject.org/linux-yocto-contrib.git tzanussi/hist-trigger-kasan-fixes
http://git.yoctoproject.org/cgit/cgit.cgi/linux-yocto-contrib/log/?h=tzanussi/hist-trigger-kasan-fixes

Steven Rostedt (1):
tracing: Fix use-after-free in hist_unreg_all/hist_enable_unreg_all

Tom Zanussi (1):
tracing: Fix use-after-free in hist_register_trigger()

 kernel/trace/trace_events_hist.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

1.9.3

From: Steven Rostedt <[email protected]>

While running tools/testing/selftests test suite with KASAN, Dmitry Vyukov hit the following use-after-free report:

==================================================================

  BUG: KASAN: use-after-free in hist_unreg_all+0x1a1/0x1d0 at addr
  ffff880031632cc0
  Read of size 8 by task ftracetest/7413
  ==================================================================
  BUG kmalloc-128 (Not tainted): kasan: bad access detected
  ------------------------------------------------------------------

This fixes the problem, along with the same problem in hist_enable_unreg_all().

---
 kernel/trace/trace_events_hist.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

index 0c05b8a..19ae135 100644
@@ -1500,9 +1500,9 @@ static void hist_unregister_trigger(char *glob, struct event_trigger_ops *ops,
 
 static void hist_unreg_all(struct trace_event_file *file)
 {
-	struct event_trigger_data *test;
+	struct event_trigger_data *test, *n;
 
-	list_for_each_entry_rcu(test, &file->triggers, list) {
+	list_for_each_entry_safe(test, n, &file->triggers, list) {
 		if (test->cmd_ops->trigger_type == ETT_EVENT_HIST) {
 			list_del_rcu(&test->list);
 			trace_event_trigger_enable_disable(file, 0);

@@ -1699,9 +1699,9 @@ hist_enable_get_trigger_ops(char *cmd, char *param)
 
 static void hist_enable_unreg_all(struct trace_event_file *file)
 {
-	struct event_trigger_data *test;
+	struct event_trigger_data *test, *n;
 
-	list_for_each_entry_rcu(test, &file->triggers, list) {
+	list_for_each_entry_safe(test, n, &file->triggers, list) {
 		if (test->cmd_ops->trigger_type == ETT_HIST_ENABLE) {
 			list_del_rcu(&test->list);
 			update_cond_flag(file);

1.9.3

This fixes a use-after-free case flagged by KASAN; make sure the test happens before the potential free in this case.

---
 kernel/trace/trace_events_hist.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

index 19ae135..f3a960e 100644
@@ -1441,6 +1441,9 @@ static int hist_register_trigger(char *glob, struct event_trigger_ops *ops,
 		goto out;
 	}
 
+	if (hist_data->attrs->pause)
+		data->paused = true;
+
 	if (named_data) {
 		destroy_hist_data(data->private_data);
 		data->private_data = named_data->private_data;

@@ -1448,9 +1451,6 @@ static int hist_register_trigger(char *glob, struct event_trigger_ops *ops,
 		data->ops = &event_hist_trigger_named_ops;
 	}
 
-	if (hist_data->attrs->pause)
-		data->paused = true;
-
 	if (data->ops->init) {
 		ret = data->ops->init(data->ops, data);
 		if (ret < 0)

1.9.3

Hi Steve,

It looks like these two patches were never merged..

Thanks,

Tom

Because they got buried in my INBOX. :-(

-- Steve