[GIT PULL] workqueue/tracing: Copy workqueue name to buffer in trace event

* [GIT PULL] workqueue/tracing: Copy workqueue name to buffer in trace event
@ 2021-03-19 13:49 Steven Rostedt
  2021-03-20  0:09 ` pr-tracker-bot
  0 siblings, 1 reply; 2+ messages in thread
From: Steven Rostedt @ 2021-03-19 13:49 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: LKML, Ingo Molnar, Andrew Morton, Tejun Heo, Zqiang


Linus,

Fix workqueue trace event unsafe string reference

After adding a verifier to test all strings printed in trace events
to make sure they either point to a string on the ring buffer,
or to read only core kernel memory, it triggered on a workqueue
trace event. The trace event workqueue_queue_work references
the allocated name of the workqueue in the output. If the workqueue
is freed before the trace is read, then the trace will dereference
freed memory. Update the trace event to use the __string(), __assign_str(),
and __get_str() helpers to handle such cases.

I also removed the stable tag as I found the problem commit was
just added to 5.12-rc1.

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


  git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-trace.git
trace-v5.12-rc3

Tag SHA1: ebefc006c81af126b0e8247406b26c69577c675f
Head SHA1: 83b62687a05205847d627f29126a8fee3c644335


Steven Rostedt (VMware) (1):
      workqueue/tracing: Copy workqueue name to buffer in trace event

----
 include/trace/events/workqueue.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
---------------------------
commit 83b62687a05205847d627f29126a8fee3c644335
Author: Steven Rostedt (VMware) <rostedt@goodmis.org>
Date:   Thu Mar 18 11:44:08 2021 -0400

    workqueue/tracing: Copy workqueue name to buffer in trace event
    
    The trace event "workqueue_queue_work" references an unsafe string in
    dereferencing the name of the workqueue. As the name is allocated, it
    could later be freed, and the pointer to that string could stay on the
    tracing buffer. If the trace buffer is read after the string is freed, it
    will reference an unsafe pointer.
    
    I added a new verifier to make sure that all strings referenced in the
    output of the trace buffer is safe to read and this triggered on the
    workqueue_queue_work trace event:
    
    workqueue_queue_work: work struct=00000000b2b235c7 function=gc_worker workqueue=(0xffff888100051160:events_power_efficient)[UNSAFE-MEMORY] req_cpu=256 cpu=1
    workqueue_queue_work: work struct=00000000c344caec function=flush_to_ldisc workqueue=(0xffff888100054d60:events_unbound)[UNSAFE-MEMORY] req_cpu=256 cpu=4294967295
    workqueue_queue_work: work struct=00000000b2b235c7 function=gc_worker workqueue=(0xffff888100051160:events_power_efficient)[UNSAFE-MEMORY] req_cpu=256 cpu=1
    workqueue_queue_work: work struct=000000000b238b3f function=vmstat_update workqueue=(0xffff8881000c3760:mm_percpu_wq)[UNSAFE-MEMORY] req_cpu=1 cpu=1
    
    Also, if this event is read via a user space application like perf or
    trace-cmd, the name would only be an address and useless information:
    
    workqueue_queue_work: work struct=0xffff953f80b4b918 function=disk_events_workfn workqueue=ffff953f8005d378 req_cpu=8192 cpu=5
    
    Cc: Zqiang <qiang.zhang@windriver.com>
    Cc: Tejun Heo <tj@kernel.org>
    Fixes: 7bf9c4a88e3e3 ("workqueue: tracing the name of the workqueue instead of it's address")
    Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>

diff --git a/include/trace/events/workqueue.h b/include/trace/events/workqueue.h
index 970cc2ea2850..6154a2e72bce 100644
--- a/include/trace/events/workqueue.h
+++ b/include/trace/events/workqueue.h
@@ -30,7 +30,7 @@ TRACE_EVENT(workqueue_queue_work,
 	TP_STRUCT__entry(
 		__field( void *,	work	)
 		__field( void *,	function)
-		__field( const char *,	workqueue)
+		__string( workqueue,	pwq->wq->name)
 		__field( unsigned int,	req_cpu	)
 		__field( unsigned int,	cpu	)
 	),
@@ -38,13 +38,13 @@ TRACE_EVENT(workqueue_queue_work,
 	TP_fast_assign(
 		__entry->work		= work;
 		__entry->function	= work->func;
-		__entry->workqueue	= pwq->wq->name;
+		__assign_str(workqueue, pwq->wq->name);
 		__entry->req_cpu	= req_cpu;
 		__entry->cpu		= pwq->pool->cpu;
 	),
 
 	TP_printk("work struct=%p function=%ps workqueue=%s req_cpu=%u cpu=%u",
-		  __entry->work, __entry->function, __entry->workqueue,
+		  __entry->work, __entry->function, __get_str(workqueue),
 		  __entry->req_cpu, __entry->cpu)
 );
 

^ permalink raw reply related	[flat|nested] 2+ messages in thread