From: Mark Rutland <mark.rutland@arm.com>
To: linux-kernel@vger.kernel.org
Cc: mark.rutland@arm.com, mhiramat@kernel.org, rostedt@goodmis.org
Subject: [PATCH] tracepoint: Fix CFI failures with tp_stub_func
Date: Thu, 23 Mar 2023 11:40:12 +0000 [thread overview]
Message-ID: <20230323114012.2162285-1-mark.rutland@arm.com> (raw)
When CLANG_CFI is in use, using tracing will occasionally result in
CFI failures, e.g.
| CFI failure at syscall_trace_enter+0x66c/0x7d0 (target: tp_stub_func+0x0/0x2c; expected type: 0x4877830c)
| Internal error: Oops - CFI: 00000000f200823a [#1] PREEMPT SMP
| CPU: 2 PID: 118 Comm: klogd Not tainted 6.3.0-rc3-00002-gc242ea6f2f98 #2
| Hardware name: linux,dummy-virt (DT)
| pstate: 30400005 (nzCV daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
| pc : syscall_trace_enter+0x66c/0x7d0
| lr : syscall_trace_enter+0x5e8/0x7d0
| sp : ffff800015ce7d80
| x29: ffff800015ce7d80 x28: ffff000017538000 x27: 0000000000000003
| x26: ffff8000084c9454 x25: ffff00001182bd10 x24: dfff800000000000
| x23: 1fffe00002ea7001 x22: ffff00001182bd10 x21: ffff000017538008
| x20: ffff000017538000 x19: ffff800015ce7eb0 x18: 0000000000000000
| x17: 000000004877830c x16: 00000000a540670c x15: 0000000000000000
| x14: 0000000000000000 x13: 0000000000000000 x12: ff80800008039d8c
| x11: ff80800008039dd0 x10: 0000000000000000 x9 : 0000000000000000
| x8 : 0000000000000000 x7 : 0000000000000000 x6 : 0000000000000000
| x5 : 0000000000000000 x4 : 0000000000000000 x3 : 0000000000000000
| x2 : 00000000000000ce x1 : ffff800015ce7eb0 x0 : ffff800013d55000
| Call trace:
| syscall_trace_enter+0x66c/0x7d0
| el0_svc_common+0x1dc/0x268
| do_el0_svc+0x70/0x1a4
| el0_svc+0x58/0x14c
| el0t_64_sync_handler+0x84/0xf0
| el0t_64_sync+0x190/0x194
| Code: 72906191 72a90ef1 6b11021f 54000040 (d4304740)
| ---[ end trace 0000000000000000 ]---
This happens because the function prototype of tp_sub_func doesn't match
the prototype of the tracepoint function. As each tracepoint may have a
distinct prototype, it's not possible to share a common stub function.
Avoid this by creating a stub function for each tracepoint with the
correct prototype, and using these rather than tp_stub_func.
Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Masami Hiramatsu <mhiramat@kernel.org
---
include/linux/tracepoint-defs.h | 1 +
include/linux/tracepoint.h | 5 +++++
kernel/tracepoint.c | 31 ++++++++++++++-----------------
3 files changed, 20 insertions(+), 17 deletions(-)
diff --git a/include/linux/tracepoint-defs.h b/include/linux/tracepoint-defs.h
index e7c2276be33eb..a306ac7255220 100644
--- a/include/linux/tracepoint-defs.h
+++ b/include/linux/tracepoint-defs.h
@@ -35,6 +35,7 @@ struct tracepoint {
struct static_call_key *static_call_key;
void *static_call_tramp;
void *iterator;
+ void *stub;
int (*regfunc)(void);
void (*unregfunc)(void);
struct tracepoint_func __rcu *funcs;
diff --git a/include/linux/tracepoint.h b/include/linux/tracepoint.h
index 6811e43c1b5c2..1640926441910 100644
--- a/include/linux/tracepoint.h
+++ b/include/linux/tracepoint.h
@@ -303,6 +303,7 @@ static inline struct tracepoint *tracepoint_ptr_deref(tracepoint_ptr_t *p)
__section("__tracepoints_strings") = #_name; \
extern struct static_call_key STATIC_CALL_KEY(tp_func_##_name); \
int __traceiter_##_name(void *__data, proto); \
+ void __tracestub_##_name(void *, proto); \
struct tracepoint __tracepoint_##_name __used \
__section("__tracepoints") = { \
.name = __tpstrtab_##_name, \
@@ -310,6 +311,7 @@ static inline struct tracepoint *tracepoint_ptr_deref(tracepoint_ptr_t *p)
.static_call_key = &STATIC_CALL_KEY(tp_func_##_name), \
.static_call_tramp = STATIC_CALL_TRAMP_ADDR(tp_func_##_name), \
.iterator = &__traceiter_##_name, \
+ .stub = &__tracestub_##_name, \
.regfunc = _reg, \
.unregfunc = _unreg, \
.funcs = NULL }; \
@@ -330,6 +332,9 @@ static inline struct tracepoint *tracepoint_ptr_deref(tracepoint_ptr_t *p)
} \
return 0; \
} \
+ void __tracestub_##_name(void *__data, proto) \
+ { \
+ } \
DEFINE_STATIC_CALL(tp_func_##_name, __traceiter_##_name);
#define DEFINE_TRACE(name, proto, args) \
diff --git a/kernel/tracepoint.c b/kernel/tracepoint.c
index 8d1507dd07246..d9186629a0095 100644
--- a/kernel/tracepoint.c
+++ b/kernel/tracepoint.c
@@ -98,12 +98,6 @@ struct tp_probes {
struct tracepoint_func probes[];
};
-/* Called in removal of a func but failed to allocate a new tp_funcs */
-static void tp_stub_func(void)
-{
- return;
-}
-
static inline void *allocate_probes(int count)
{
struct tp_probes *p = kmalloc(struct_size(p, probes, count),
@@ -177,13 +171,15 @@ static void debug_print_probes(struct tracepoint_func *funcs)
}
static struct tracepoint_func *
-func_add(struct tracepoint_func **funcs, struct tracepoint_func *tp_func,
+func_add(struct tracepoint *tp, struct tracepoint_func **funcs,
+ struct tracepoint_func *tp_func,
int prio)
{
struct tracepoint_func *old, *new;
int iter_probes; /* Iterate over old probe array. */
int nr_probes = 0; /* Counter for probes */
int pos = -1; /* Insertion position into new array */
+ void *stub_func = tp->stub;
if (WARN_ON(!tp_func->func))
return ERR_PTR(-EINVAL);
@@ -193,7 +189,7 @@ func_add(struct tracepoint_func **funcs, struct tracepoint_func *tp_func,
if (old) {
/* (N -> N+1), (N != 0, 1) probes */
for (iter_probes = 0; old[iter_probes].func; iter_probes++) {
- if (old[iter_probes].func == tp_stub_func)
+ if (old[iter_probes].func == stub_func)
continue; /* Skip stub functions. */
if (old[iter_probes].func == tp_func->func &&
old[iter_probes].data == tp_func->data)
@@ -208,7 +204,7 @@ func_add(struct tracepoint_func **funcs, struct tracepoint_func *tp_func,
if (old) {
nr_probes = 0;
for (iter_probes = 0; old[iter_probes].func; iter_probes++) {
- if (old[iter_probes].func == tp_stub_func)
+ if (old[iter_probes].func == stub_func)
continue;
/* Insert before probes of lower priority */
if (pos < 0 && old[iter_probes].prio < prio)
@@ -229,11 +225,12 @@ func_add(struct tracepoint_func **funcs, struct tracepoint_func *tp_func,
return old;
}
-static void *func_remove(struct tracepoint_func **funcs,
- struct tracepoint_func *tp_func)
+static void *func_remove(struct tracepoint *tp, struct tracepoint_func **funcs,
+ struct tracepoint_func *tp_func)
{
int nr_probes = 0, nr_del = 0, i;
struct tracepoint_func *old, *new;
+ void *stub_func = tp->stub;
old = *funcs;
@@ -246,7 +243,7 @@ static void *func_remove(struct tracepoint_func **funcs,
for (nr_probes = 0; old[nr_probes].func; nr_probes++) {
if ((old[nr_probes].func == tp_func->func &&
old[nr_probes].data == tp_func->data) ||
- old[nr_probes].func == tp_stub_func)
+ old[nr_probes].func == stub_func)
nr_del++;
}
}
@@ -269,7 +266,7 @@ static void *func_remove(struct tracepoint_func **funcs,
for (i = 0; old[i].func; i++) {
if ((old[i].func != tp_func->func ||
old[i].data != tp_func->data) &&
- old[i].func != tp_stub_func)
+ old[i].func != stub_func)
new[j++] = old[i];
}
new[nr_probes - nr_del].func = NULL;
@@ -277,12 +274,12 @@ static void *func_remove(struct tracepoint_func **funcs,
} else {
/*
* Failed to allocate, replace the old function
- * with calls to tp_stub_func.
+ * with calls to stub_func.
*/
for (i = 0; old[i].func; i++) {
if (old[i].func == tp_func->func &&
old[i].data == tp_func->data)
- WRITE_ONCE(old[i].func, tp_stub_func);
+ WRITE_ONCE(old[i].func, stub_func);
}
*funcs = old;
}
@@ -335,7 +332,7 @@ static int tracepoint_add_func(struct tracepoint *tp,
tp_funcs = rcu_dereference_protected(tp->funcs,
lockdep_is_held(&tracepoints_mutex));
- old = func_add(&tp_funcs, func, prio);
+ old = func_add(tp, &tp_funcs, func, prio);
if (IS_ERR(old)) {
WARN_ON_ONCE(warn && PTR_ERR(old) != -ENOMEM);
return PTR_ERR(old);
@@ -400,7 +397,7 @@ static int tracepoint_remove_func(struct tracepoint *tp,
tp_funcs = rcu_dereference_protected(tp->funcs,
lockdep_is_held(&tracepoints_mutex));
- old = func_remove(&tp_funcs, func);
+ old = func_remove(tp, &tp_funcs, func);
if (WARN_ON_ONCE(IS_ERR(old)))
return PTR_ERR(old);
--
2.30.2
next reply other threads:[~2023-03-23 11:40 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-03-23 11:40 Mark Rutland [this message]
2023-03-23 12:53 ` Steven Rostedt
2023-03-23 13:45 ` Mark Rutland
2023-03-23 16:27 ` Mark Rutland
2023-03-23 16:34 ` Steven Rostedt
2023-03-23 18:37 ` Mathieu Desnoyers
2023-03-23 18:42 ` Steven Rostedt
2023-03-23 18:44 ` Mathieu Desnoyers
2023-03-23 16:26 ` Steven Rostedt
2023-03-23 16:32 ` Mark Rutland
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20230323114012.2162285-1-mark.rutland@arm.com \
--to=mark.rutland@arm.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mhiramat@kernel.org \
--cc=rostedt@goodmis.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®