Re: [PATCH V2] tracing: Fix race between update_event_fields and event_define_fields
From: Michael Wu
Date: Thu Jul 30 2026 - 07:21:50 EST
Hi Masami,
Thanks for the review. I've rebased the patch. The new version is below.
---
From ec00b31c58b56f4cbbe6dd67c9f88a1d87d8d5ac Mon Sep 17 00:00:00 2001
From: Michael Wu <michael@xxxxxxxxxxxxxxxxx>
Date: Wed, 29 Jul 2026 17:34:09 +0800
Subject: [PATCH v2] tracing: Fix race between update_event_fields and
event_define_fields
event_define_fields() (pri=1 MODULE_STATE_COMING notifier, locked by
event_mutex) populates class->fields via list_add(), while
update_event_fields() (called from the pri=0 notifier path via
trace_event_update_all) traverses class->fields protected only by
trace_event_sem. These are two different locks guarding the same
data structure, so during cross-module loading a reader on one CPU can
observe partially initialized list nodes being concurrently added by a
writer on another CPU.
On arm64 with weak memory ordering, __list_add() writes to two
different cache lines:
next->prev = new; // (1) ordinary store
new->next = next; // (2) ordinary store — field's cache line
new->prev = prev; // (3) ordinary store
WRITE_ONCE(prev->next, new); // (4) release store — head's cache line
The store buffer can drain (2) and (4) independently since they target
different cache lines. A remote CPU may observe (4) before (2): it
sees prev->next pointing to the new node, but the new node's link.next
is still zero (kmem_cache_alloc zero-initialized via KMEM_CACHE with
SLAB_PANIC). Since offsetof(struct ftrace_event_field, link) == 0,
list_for_each_entry() derives field == NULL from link.next == 0 and
crashes at field->type (offset 0x18):
Unable to handle kernel access ... at virtual address 0000000000000018
pc : update_event_fields+0xf8/0x368
Call trace:
update_event_fields+0xf8/0x368
trace_event_update_all+0x7c/0x2b4
trace_module_notify+0x4c/0x1dc
notifier_call_chain+0x84/0x168
blocking_notifier_call_chain_robust+0x64/0xd4
load_module+0x10c8/0x123c
__arm64_sys_finit_module+0x230/0x31c
Fix by holding event_mutex in trace_event_update_all() before
down_write(&trace_event_sem). This serializes the reader against
event_define_fields() which takes event_mutex for the entire
write-side critical section. The lock ordering event_mutex ->
trace_event_sem is already established at three other sites
(trace_remove_event_call, event_trace_add_tracer,
event_trace_del_tracer), so this introduces no ordering conflict.
An alternative of placing mutex_lock inside update_event_fields() was
considered but rejected — it would invert the established lock
ordering (trace_event_sem -> event_mutex) and risk ABBA deadlock.
Fixes: b3bc8547d3be ("tracing: Have TRACE_DEFINE_ENUM affect trace event types as well")
Cc: stable@xxxxxxxxxxxxxxx
Signed-off-by: Michael Wu <michael@xxxxxxxxxxxxxxxxx>
---
kernel/trace/trace_events.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
index 956692856fa8..9632788da5af 100644
--- a/kernel/trace/trace_events.c
+++ b/kernel/trace/trace_events.c
@@ -3566,6 +3566,7 @@ void trace_event_update_all(struct trace_eval_map **map, int len)
int last_i;
int i;
+ mutex_lock(&event_mutex);
down_write(&trace_event_sem);
list_for_each_entry_safe(call, p, &ftrace_events, list) {
/* events are usually grouped together with systems */
@@ -3604,6 +3605,7 @@ void trace_event_update_all(struct trace_eval_map **map, int len)
cond_resched();
}
up_write(&trace_event_sem);
+ mutex_unlock(&event_mutex);
}
static bool event_in_systems(struct trace_event_call *call,
--
2.29.0
On 7/30/2026 6:54 PM, Michael Wu wrote:
event_define_fields() (pri=1 MODULE_STATE_COMING notifier, locked by
event_mutex) populates class->fields via list_add(), while
update_event_fields() (called from the pri=0 notifier path via
trace_event_update_all) traverses class->fields protected only by
trace_event_sem. These are two different locks guarding the same
data structure, so during cross-module loading a reader on one CPU can
observe partially initialized list nodes being concurrently added by a
writer on another CPU.
On arm64 with weak memory ordering, __list_add() writes to two
different cache lines:
next->prev = new; // (1) ordinary store
new->next = next; // (2) ordinary store — field's cache line
new->prev = prev; // (3) ordinary store
WRITE_ONCE(prev->next, new); // (4) release store — head's cache line
The store buffer can drain (2) and (4) independently since they target
different cache lines. A remote CPU may observe (4) before (2): it
sees prev->next pointing to the new node, but the new node's link.next
is still zero (kmem_cache_alloc zero-initialized via KMEM_CACHE with
SLAB_PANIC). Since offsetof(struct ftrace_event_field, link) == 0,
list_for_each_entry() derives field == NULL from link.next == 0 and
crashes at field->type (offset 0x18):
Unable to handle kernel access ... at virtual address 0000000000000018
pc : update_event_fields+0xf8/0x368
Call trace:
update_event_fields+0xf8/0x368
trace_event_update_all+0x7c/0x2b4
trace_module_notify+0x4c/0x1dc
notifier_call_chain+0x84/0x168
blocking_notifier_call_chain_robust+0x64/0xd4
load_module+0x10c8/0x123c
__arm64_sys_finit_module+0x230/0x31c
Fix by holding event_mutex in trace_event_update_all() before
down_write(&trace_event_sem). This serializes the reader against
event_define_fields() which takes event_mutex for the entire
write-side critical section. The lock ordering event_mutex ->
trace_event_sem is already established at three other sites
(trace_remove_event_call, event_trace_add_tracer,
event_trace_del_tracer), so this introduces no ordering conflict.
An alternative of placing mutex_lock inside update_event_fields() was
considered but rejected — it would invert the established lock
ordering (trace_event_sem -> event_mutex) and risk ABBA deadlock.
Fixes: b3bc8547d3be ("tracing: Have TRACE_DEFINE_ENUM affect trace event types as well")
Cc: stable@xxxxxxxxxxxxxxx
---
kernel/trace/trace_events.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
index 956692856fa8..9632788da5af 100644
--- a/kernel/trace/trace_events.c
+++ b/kernel/trace/trace_events.c
@@ -3566,6 +3566,7 @@ void trace_event_update_all(struct trace_eval_map **map, int len)
int last_i;
int i;
+ mutex_lock(&event_mutex);
down_write(&trace_event_sem);
list_for_each_entry_safe(call, p, &ftrace_events, list) {
/* events are usually grouped together with systems */
@@ -3604,6 +3605,7 @@ void trace_event_update_all(struct trace_eval_map **map, int len)
cond_resched();
}
up_write(&trace_event_sem);
+ mutex_unlock(&event_mutex);
}
static bool event_in_systems(struct trace_event_call *call,
--
Regards,
Michael Wu