[PATCH 0/2] tracing: hist trigger KASAN fixes

[PATCH 0/2] tracing: hist trigger KASAN fixes

[Tom Zanussi]

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

[PATCH 1/2] tracing: Fix use-after-free in hist_unreg_all/hist_enable_unreg_all

[Tom Zanussi]

From: Steven Rostedt <rostedt@goodmis.org>

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().

Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
[Copied Steve's hist_enable_unreg_all() fix to hist_unreg_all()]
Signed-off-by: Tom Zanussi <tom.zanussi@linux.intel.com>
Cc: Dmitry Vyukov <dvyukov@google.com>
---
 kernel/trace/trace_events_hist.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c
index 0c05b8a..19ae135 100644
--- a/kernel/trace/trace_events_hist.c
+++ b/kernel/trace/trace_events_hist.c
@@ -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

[PATCH 2/2] tracing: Fix use-after-free in hist_register_trigger()

[Tom Zanussi]

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

Signed-off-by: Tom Zanussi <tom.zanussi@linux.intel.com>
---
 kernel/trace/trace_events_hist.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c
index 19ae135..f3a960e 100644
--- a/kernel/trace/trace_events_hist.c
+++ b/kernel/trace/trace_events_hist.c
@@ -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

Re: [PATCH 0/2] tracing: hist trigger KASAN fixes

[Tom Zanussi]

Hi Steve,

It looks like these two patches were never merged..

Thanks,

Tom

On 06/29/2016 07:55 PM, Tom Zanussi wrote:
> 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(-)
> 

Re: [PATCH 0/2] tracing: hist trigger KASAN fixes

[Steven Rostedt]

On Tue, 02 Aug 2016 13:57:13 -0500
Tom Zanussi <tom.zanussi@linux.intel.com> wrote:

> Hi Steve,
> 
> It looks like these two patches were never merged..
> 

Because they got buried in my INBOX. :-(

-- Steve