* [for-linus][PATCH 1/7] ftrace: Fix NULL pointer dereference in t_probe_next()
2019-08-31 10:53 [for-linus][PATCH 0/7] tracing: Updates for 5.3 Steven Rostedt
@ 2019-08-31 10:53 ` Steven Rostedt
2019-08-31 10:53 ` [for-linus][PATCH 2/7] ftrace: Check for empty hash and comment the race with registering probes Steven Rostedt
` (5 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: Steven Rostedt @ 2019-08-31 10:53 UTC (permalink / raw
To: linux-kernel; +Cc: Ingo Molnar, Andrew Morton, stable, Naveen N. Rao
From: "Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com>
LTP testsuite on powerpc results in the below crash:
Unable to handle kernel paging request for data at address 0x00000000
Faulting instruction address: 0xc00000000029d800
Oops: Kernel access of bad area, sig: 11 [#1]
LE SMP NR_CPUS=2048 NUMA PowerNV
...
CPU: 68 PID: 96584 Comm: cat Kdump: loaded Tainted: G W
NIP: c00000000029d800 LR: c00000000029dac4 CTR: c0000000001e6ad0
REGS: c0002017fae8ba10 TRAP: 0300 Tainted: G W
MSR: 9000000000009033 <SF,HV,EE,ME,IR,DR,RI,LE> CR: 28022422 XER: 20040000
CFAR: c00000000029d90c DAR: 0000000000000000 DSISR: 40000000 IRQMASK: 0
...
NIP [c00000000029d800] t_probe_next+0x60/0x180
LR [c00000000029dac4] t_mod_start+0x1a4/0x1f0
Call Trace:
[c0002017fae8bc90] [c000000000cdbc40] _cond_resched+0x10/0xb0 (unreliable)
[c0002017fae8bce0] [c0000000002a15b0] t_start+0xf0/0x1c0
[c0002017fae8bd30] [c0000000004ec2b4] seq_read+0x184/0x640
[c0002017fae8bdd0] [c0000000004a57bc] sys_read+0x10c/0x300
[c0002017fae8be30] [c00000000000b388] system_call+0x5c/0x70
The test (ftrace_set_ftrace_filter.sh) is part of ftrace stress tests
and the crash happens when the test does 'cat
$TRACING_PATH/set_ftrace_filter'.
The address points to the second line below, in t_probe_next(), where
filter_hash is dereferenced:
hash = iter->probe->ops.func_hash->filter_hash;
size = 1 << hash->size_bits;
This happens due to a race with register_ftrace_function_probe(). A new
ftrace_func_probe is created and added into the func_probes list in
trace_array under ftrace_lock. However, before initializing the filter,
we drop ftrace_lock, and re-acquire it after acquiring regex_lock. If
another process is trying to read set_ftrace_filter, it will be able to
acquire ftrace_lock during this window and it will end up seeing a NULL
filter_hash.
Fix this by just checking for a NULL filter_hash in t_probe_next(). If
the filter_hash is NULL, then this probe is just being added and we can
simply return from here.
Link: http://lkml.kernel.org/r/05e021f757625cbbb006fad41380323dbe4e3b43.1562249521.git.naveen.n.rao@linux.vnet.ibm.com
Cc: stable@vger.kernel.org
Fixes: 7b60f3d876156 ("ftrace: Dynamically create the probe ftrace_ops for the trace_array")
Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
kernel/trace/ftrace.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index eca34503f178..80beed2cf0da 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -3095,6 +3095,10 @@ t_probe_next(struct seq_file *m, loff_t *pos)
hnd = &iter->probe_entry->hlist;
hash = iter->probe->ops.func_hash->filter_hash;
+
+ if (!hash)
+ return NULL;
+
size = 1 << hash->size_bits;
retry:
--
2.20.1
^ permalink raw reply related [flat|nested] 8+ messages in thread* [for-linus][PATCH 2/7] ftrace: Check for empty hash and comment the race with registering probes
2019-08-31 10:53 [for-linus][PATCH 0/7] tracing: Updates for 5.3 Steven Rostedt
2019-08-31 10:53 ` [for-linus][PATCH 1/7] ftrace: Fix NULL pointer dereference in t_probe_next() Steven Rostedt
@ 2019-08-31 10:53 ` Steven Rostedt
2019-08-31 10:53 ` [for-linus][PATCH 3/7] ftrace: Check for successful allocation of hash Steven Rostedt
` (4 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: Steven Rostedt @ 2019-08-31 10:53 UTC (permalink / raw
To: linux-kernel; +Cc: Ingo Molnar, Andrew Morton, stable
From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>
The race between adding a function probe and reading the probes that exist
is very subtle. It needs a comment. Also, the issue can also happen if the
probe has has the EMPTY_HASH as its func_hash.
Cc: stable@vger.kernel.org
Fixes: 7b60f3d876156 ("ftrace: Dynamically create the probe ftrace_ops for the trace_array")
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
kernel/trace/ftrace.c | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 80beed2cf0da..6200a6fe10e3 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -3096,7 +3096,11 @@ t_probe_next(struct seq_file *m, loff_t *pos)
hash = iter->probe->ops.func_hash->filter_hash;
- if (!hash)
+ /*
+ * A probe being registered may temporarily have an empty hash
+ * and it's at the end of the func_probes list.
+ */
+ if (!hash || hash == EMPTY_HASH)
return NULL;
size = 1 << hash->size_bits;
@@ -4324,6 +4328,10 @@ register_ftrace_function_probe(char *glob, struct trace_array *tr,
mutex_unlock(&ftrace_lock);
+ /*
+ * Note, there's a small window here that the func_hash->filter_hash
+ * may be NULL or empty. Need to be carefule when reading the loop.
+ */
mutex_lock(&probe->ops.func_hash->regex_lock);
orig_hash = &probe->ops.func_hash->filter_hash;
--
2.20.1
^ permalink raw reply related [flat|nested] 8+ messages in thread* [for-linus][PATCH 3/7] ftrace: Check for successful allocation of hash
2019-08-31 10:53 [for-linus][PATCH 0/7] tracing: Updates for 5.3 Steven Rostedt
2019-08-31 10:53 ` [for-linus][PATCH 1/7] ftrace: Fix NULL pointer dereference in t_probe_next() Steven Rostedt
2019-08-31 10:53 ` [for-linus][PATCH 2/7] ftrace: Check for empty hash and comment the race with registering probes Steven Rostedt
@ 2019-08-31 10:53 ` Steven Rostedt
2019-08-31 10:53 ` [for-linus][PATCH 4/7] tracing: Make exported ftrace_set_clr_event non-static Steven Rostedt
` (3 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: Steven Rostedt @ 2019-08-31 10:53 UTC (permalink / raw
To: linux-kernel; +Cc: Ingo Molnar, Andrew Morton, stable, Naveen N. Rao
From: "Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com>
In register_ftrace_function_probe(), we are not checking the return
value of alloc_and_copy_ftrace_hash(). The subsequent call to
ftrace_match_records() may end up dereferencing the same. Add a check to
ensure this doesn't happen.
Link: http://lkml.kernel.org/r/26e92574f25ad23e7cafa3cf5f7a819de1832cbe.1562249521.git.naveen.n.rao@linux.vnet.ibm.com
Cc: stable@vger.kernel.org
Fixes: 1ec3a81a0cf42 ("ftrace: Have each function probe use its own ftrace_ops")
Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
kernel/trace/ftrace.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 6200a6fe10e3..f9821a3374e9 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -4338,6 +4338,11 @@ register_ftrace_function_probe(char *glob, struct trace_array *tr,
old_hash = *orig_hash;
hash = alloc_and_copy_ftrace_hash(FTRACE_HASH_DEFAULT_BITS, old_hash);
+ if (!hash) {
+ ret = -ENOMEM;
+ goto out;
+ }
+
ret = ftrace_match_records(hash, glob, strlen(glob));
/* Nothing found? */
--
2.20.1
^ permalink raw reply related [flat|nested] 8+ messages in thread* [for-linus][PATCH 4/7] tracing: Make exported ftrace_set_clr_event non-static
2019-08-31 10:53 [for-linus][PATCH 0/7] tracing: Updates for 5.3 Steven Rostedt
` (2 preceding siblings ...)
2019-08-31 10:53 ` [for-linus][PATCH 3/7] ftrace: Check for successful allocation of hash Steven Rostedt
@ 2019-08-31 10:53 ` Steven Rostedt
2019-08-31 10:53 ` [for-linus][PATCH 5/7] tracing/probe: Fix null pointer dereference Steven Rostedt
` (2 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: Steven Rostedt @ 2019-08-31 10:53 UTC (permalink / raw
To: linux-kernel; +Cc: Ingo Molnar, Andrew Morton, Joe Jin, Denis Efremov
From: Denis Efremov <efremov@linux.com>
The function ftrace_set_clr_event is declared static and marked
EXPORT_SYMBOL_GPL(), which is at best an odd combination. Because the
function was decided to be a part of API, this commit removes the static
attribute and adds the declaration to the header.
Link: http://lkml.kernel.org/r/20190704172110.27041-1-efremov@linux.com
Fixes: f45d1225adb04 ("tracing: Kernel access to Ftrace instances")
Reviewed-by: Joe Jin <joe.jin@oracle.com>
Signed-off-by: Denis Efremov <efremov@linux.com>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
include/linux/trace_events.h | 1 +
kernel/trace/trace_events.c | 2 +-
2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/include/linux/trace_events.h b/include/linux/trace_events.h
index 5150436783e8..30a8cdcfd4a4 100644
--- a/include/linux/trace_events.h
+++ b/include/linux/trace_events.h
@@ -548,6 +548,7 @@ extern int trace_event_get_offsets(struct trace_event_call *call);
#define is_signed_type(type) (((type)(-1)) < (type)1)
+int ftrace_set_clr_event(struct trace_array *tr, char *buf, int set);
int trace_set_clr_event(const char *system, const char *event, int set);
/*
diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
index c7506bc81b75..648930823b57 100644
--- a/kernel/trace/trace_events.c
+++ b/kernel/trace/trace_events.c
@@ -787,7 +787,7 @@ static int __ftrace_set_clr_event(struct trace_array *tr, const char *match,
return ret;
}
-static int ftrace_set_clr_event(struct trace_array *tr, char *buf, int set)
+int ftrace_set_clr_event(struct trace_array *tr, char *buf, int set)
{
char *event = NULL, *sub = NULL, *match;
int ret;
--
2.20.1
^ permalink raw reply related [flat|nested] 8+ messages in thread* [for-linus][PATCH 5/7] tracing/probe: Fix null pointer dereference
2019-08-31 10:53 [for-linus][PATCH 0/7] tracing: Updates for 5.3 Steven Rostedt
` (3 preceding siblings ...)
2019-08-31 10:53 ` [for-linus][PATCH 4/7] tracing: Make exported ftrace_set_clr_event non-static Steven Rostedt
@ 2019-08-31 10:53 ` Steven Rostedt
2019-08-31 10:53 ` [for-linus][PATCH 6/7] ftrace/x86: Remove mcount() declaration Steven Rostedt
2019-08-31 10:53 ` [for-linus][PATCH 7/7] tracing: Correct kdoc formats Steven Rostedt
6 siblings, 0 replies; 8+ messages in thread
From: Steven Rostedt @ 2019-08-31 10:53 UTC (permalink / raw
To: linux-kernel; +Cc: Ingo Molnar, Andrew Morton, Xinpeng Liu
From: Xinpeng Liu <danielliu861@gmail.com>
BUG: KASAN: null-ptr-deref in trace_probe_cleanup+0x8d/0xd0
Read of size 8 at addr 0000000000000000 by task syz-executor.0/9746
trace_probe_cleanup+0x8d/0xd0
free_trace_kprobe.part.14+0x15/0x50
alloc_trace_kprobe+0x23e/0x250
Link: http://lkml.kernel.org/r/1565220563-980-1-git-send-email-danielliu861@gmail.com
Fixes: e3dc9f898ef9c ("tracing/probe: Add trace_event_call accesses APIs")
Signed-off-by: Xinpeng Liu <danielliu861@gmail.com>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
kernel/trace/trace_probe.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c
index dbef0d135075..fb6bfbc5bf86 100644
--- a/kernel/trace/trace_probe.c
+++ b/kernel/trace/trace_probe.c
@@ -895,7 +895,8 @@ void trace_probe_cleanup(struct trace_probe *tp)
for (i = 0; i < tp->nr_args; i++)
traceprobe_free_probe_arg(&tp->args[i]);
- kfree(call->class->system);
+ if (call->class)
+ kfree(call->class->system);
kfree(call->name);
kfree(call->print_fmt);
}
--
2.20.1
^ permalink raw reply related [flat|nested] 8+ messages in thread* [for-linus][PATCH 6/7] ftrace/x86: Remove mcount() declaration
2019-08-31 10:53 [for-linus][PATCH 0/7] tracing: Updates for 5.3 Steven Rostedt
` (4 preceding siblings ...)
2019-08-31 10:53 ` [for-linus][PATCH 5/7] tracing/probe: Fix null pointer dereference Steven Rostedt
@ 2019-08-31 10:53 ` Steven Rostedt
2019-08-31 10:53 ` [for-linus][PATCH 7/7] tracing: Correct kdoc formats Steven Rostedt
6 siblings, 0 replies; 8+ messages in thread
From: Steven Rostedt @ 2019-08-31 10:53 UTC (permalink / raw
To: linux-kernel; +Cc: Ingo Molnar, Andrew Morton, Jisheng Zhang
From: Jisheng Zhang <Jisheng.Zhang@synaptics.com>
Commit 562e14f72292 ("ftrace/x86: Remove mcount support") removed the
support for using mcount, so we could remove the mcount() declaration
to clean up.
Link: http://lkml.kernel.org/r/20190826170150.10f101ba@xhacker.debian
Signed-off-by: Jisheng Zhang <Jisheng.Zhang@synaptics.com>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
arch/x86/include/asm/ftrace.h | 1 -
1 file changed, 1 deletion(-)
diff --git a/arch/x86/include/asm/ftrace.h b/arch/x86/include/asm/ftrace.h
index 287f1f7b2e52..c38a66661576 100644
--- a/arch/x86/include/asm/ftrace.h
+++ b/arch/x86/include/asm/ftrace.h
@@ -16,7 +16,6 @@
#define HAVE_FUNCTION_GRAPH_RET_ADDR_PTR
#ifndef __ASSEMBLY__
-extern void mcount(void);
extern atomic_t modifying_ftrace_code;
extern void __fentry__(void);
--
2.20.1
^ permalink raw reply related [flat|nested] 8+ messages in thread* [for-linus][PATCH 7/7] tracing: Correct kdoc formats
2019-08-31 10:53 [for-linus][PATCH 0/7] tracing: Updates for 5.3 Steven Rostedt
` (5 preceding siblings ...)
2019-08-31 10:53 ` [for-linus][PATCH 6/7] ftrace/x86: Remove mcount() declaration Steven Rostedt
@ 2019-08-31 10:53 ` Steven Rostedt
6 siblings, 0 replies; 8+ messages in thread
From: Steven Rostedt @ 2019-08-31 10:53 UTC (permalink / raw
To: linux-kernel; +Cc: Ingo Molnar, Andrew Morton, Jakub Kicinski
From: Jakub Kicinski <jakub.kicinski@netronome.com>
Fix the following kdoc warnings:
kernel/trace/trace.c:1579: warning: Function parameter or member 'tr' not described in 'update_max_tr_single'
kernel/trace/trace.c:1579: warning: Function parameter or member 'tsk' not described in 'update_max_tr_single'
kernel/trace/trace.c:1579: warning: Function parameter or member 'cpu' not described in 'update_max_tr_single'
kernel/trace/trace.c:1776: warning: Function parameter or member 'type' not described in 'register_tracer'
kernel/trace/trace.c:2239: warning: Function parameter or member 'task' not described in 'tracing_record_taskinfo'
kernel/trace/trace.c:2239: warning: Function parameter or member 'flags' not described in 'tracing_record_taskinfo'
kernel/trace/trace.c:2269: warning: Function parameter or member 'prev' not described in 'tracing_record_taskinfo_sched_switch'
kernel/trace/trace.c:2269: warning: Function parameter or member 'next' not described in 'tracing_record_taskinfo_sched_switch'
kernel/trace/trace.c:2269: warning: Function parameter or member 'flags' not described in 'tracing_record_taskinfo_sched_switch'
kernel/trace/trace.c:3078: warning: Function parameter or member 'ip' not described in 'trace_vbprintk'
kernel/trace/trace.c:3078: warning: Function parameter or member 'fmt' not described in 'trace_vbprintk'
kernel/trace/trace.c:3078: warning: Function parameter or member 'args' not described in 'trace_vbprintk'
Link: http://lkml.kernel.org/r/20190828052549.2472-2-jakub.kicinski@netronome.com
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
kernel/trace/trace.c | 26 ++++++++++++++------------
1 file changed, 14 insertions(+), 12 deletions(-)
diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 525a97fbbc60..563e80f9006a 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -1567,9 +1567,9 @@ update_max_tr(struct trace_array *tr, struct task_struct *tsk, int cpu,
/**
* update_max_tr_single - only copy one trace over, and reset the rest
- * @tr - tracer
- * @tsk - task with the latency
- * @cpu - the cpu of the buffer to copy.
+ * @tr: tracer
+ * @tsk: task with the latency
+ * @cpu: the cpu of the buffer to copy.
*
* Flip the trace of a single CPU buffer between the @tr and the max_tr.
*/
@@ -1767,7 +1767,7 @@ static void __init apply_trace_boot_options(void);
/**
* register_tracer - register a tracer with the ftrace system.
- * @type - the plugin for the tracer
+ * @type: the plugin for the tracer
*
* Register a new plugin tracer.
*/
@@ -2230,9 +2230,9 @@ static bool tracing_record_taskinfo_skip(int flags)
/**
* tracing_record_taskinfo - record the task info of a task
*
- * @task - task to record
- * @flags - TRACE_RECORD_CMDLINE for recording comm
- * - TRACE_RECORD_TGID for recording tgid
+ * @task: task to record
+ * @flags: TRACE_RECORD_CMDLINE for recording comm
+ * TRACE_RECORD_TGID for recording tgid
*/
void tracing_record_taskinfo(struct task_struct *task, int flags)
{
@@ -2258,10 +2258,10 @@ void tracing_record_taskinfo(struct task_struct *task, int flags)
/**
* tracing_record_taskinfo_sched_switch - record task info for sched_switch
*
- * @prev - previous task during sched_switch
- * @next - next task during sched_switch
- * @flags - TRACE_RECORD_CMDLINE for recording comm
- * TRACE_RECORD_TGID for recording tgid
+ * @prev: previous task during sched_switch
+ * @next: next task during sched_switch
+ * @flags: TRACE_RECORD_CMDLINE for recording comm
+ * TRACE_RECORD_TGID for recording tgid
*/
void tracing_record_taskinfo_sched_switch(struct task_struct *prev,
struct task_struct *next, int flags)
@@ -3072,7 +3072,9 @@ static void trace_printk_start_stop_comm(int enabled)
/**
* trace_vbprintk - write binary msg to tracing buffer
- *
+ * @ip: The address of the caller
+ * @fmt: The string format to write to the buffer
+ * @args: Arguments for @fmt
*/
int trace_vbprintk(unsigned long ip, const char *fmt, va_list args)
{
--
2.20.1
^ permalink raw reply related [flat|nested] 8+ messages in thread