linux-kernel - [GIT PULL] tracing/samples: Fix creation and deletion of simple_thread

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-ID: <20171017172150.3ff71046@gandalf.local.home>
Date:   Tue, 17 Oct 2017 17:21:50 -0400
From:   Steven Rostedt <rostedt@...dmis.org>
To:     Linus Torvalds <torvalds@...ux-foundation.org>
Cc:     LKML <linux-kernel@...r.kernel.org>,
        Ingo Molnar <mingo@...nel.org>,
        Andrew Morton <akpm@...ux-foundation.org>,
        stable <stable@...r.kernel.org>
Subject: [GIT PULL] tracing/samples: Fix creation and deletion of
 simple_thread_fn creation


Linus,

Testing a new trace event format, I triggered a bug by doing:

  # modprobe trace-events-sample
  # echo 1 > /sys/kernel/debug/tracing/events/sample-trace/enable
  # rmmod trace-events-sample

This would cause an oops. The issue is that I added another trace
event sample that reused a reg function of another trace event to
create a thread to call the tracepoints. The problem was that the
reg function couldn't handle nested calls (reg; reg; unreg; unreg;)
and created two threads (instead of one) and only removed one
on exit.

This isn't a critical bug as the bug is only in sample code. But sample
code should be free of known bugs to prevent others from copying
it. This is why this is also marked for stable.

[ Please note, my subkey just expired and I just created a new one
  which signed this pull request ]

Please pull the latest trace-v4.14-rc3 tree, which can be found at:


  git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-trace.git
trace-v4.14-rc3

Tag SHA1: 0c38cf14b856c50632b3fe36d8607f4f2e812e52
Head SHA1: 6575257c60e1a26a5319ccf2b5ce5b6449001017


Steven Rostedt (VMware) (1):
      tracing/samples: Fix creation and deletion of simple_thread_fn creation

----
 samples/trace_events/trace-events-sample.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)
---------------------------
commit 6575257c60e1a26a5319ccf2b5ce5b6449001017
Author: Steven Rostedt (VMware) <rostedt@...dmis.org>
Date:   Tue Oct 17 14:55:24 2017 -0400

    tracing/samples: Fix creation and deletion of simple_thread_fn creation
    
    Commit 7496946a8 ("tracing: Add samples of DECLARE_EVENT_CLASS() and
    DEFINE_EVENT()") added template examples for all the events. It created a
    DEFINE_EVENT_FN() example which reused the foo_bar_reg and foo_bar_unreg
    functions.
    
    Enabling both the TRACE_EVENT_FN() and DEFINE_EVENT_FN() example trace
    events caused the foo_bar_reg to be called twice, creating the test thread
    twice. The foo_bar_unreg would remove it only once, even if it was called
    multiple times, leaving a thread existing when the module is unloaded,
    causing an oops.
    
    Add a ref count and allow foo_bar_reg() and foo_bar_unreg() be called by
    multiple trace events.
    
    Cc: stable@...r.kernel.org
    Fixes: 7496946a8 ("tracing: Add samples of DECLARE_EVENT_CLASS() and DEFINE_EVENT()")
    Signed-off-by: Steven Rostedt (VMware) <rostedt@...dmis.org>

diff --git a/samples/trace_events/trace-events-sample.c b/samples/trace_events/trace-events-sample.c
index bc7fcf010a5b..446beb7ac48d 100644
--- a/samples/trace_events/trace-events-sample.c
+++ b/samples/trace_events/trace-events-sample.c
@@ -78,29 +78,37 @@ static int simple_thread_fn(void *arg)
 }
 
 static DEFINE_MUTEX(thread_mutex);
+static bool simple_thread_cnt;
 
 int foo_bar_reg(void)
 {
+	mutex_lock(&thread_mutex);
+	if (simple_thread_cnt++)
+		goto out;
+
 	pr_info("Starting thread for foo_bar_fn\n");
 	/*
 	 * We shouldn't be able to start a trace when the module is
 	 * unloading (there's other locks to prevent that). But
 	 * for consistency sake, we still take the thread_mutex.
 	 */
-	mutex_lock(&thread_mutex);
 	simple_tsk_fn = kthread_run(simple_thread_fn, NULL, "event-sample-fn");
+ out:
 	mutex_unlock(&thread_mutex);
 	return 0;
 }
 
 void foo_bar_unreg(void)
 {
-	pr_info("Killing thread for foo_bar_fn\n");
-	/* protect against module unloading */
 	mutex_lock(&thread_mutex);
+	if (--simple_thread_cnt)
+		goto out;
+
+	pr_info("Killing thread for foo_bar_fn\n");
 	if (simple_tsk_fn)
 		kthread_stop(simple_tsk_fn);
 	simple_tsk_fn = NULL;
+ out:
 	mutex_unlock(&thread_mutex);
 }