All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] tracing: reject trace events with an empty TP_STRUCT__entry
@ 2026-10-16  6:57 Haoran Wang
  2026-10-16  7:08 ` sashiko-bot
  2026-10-16 14:13 ` Steven Rostedt
  0 siblings, 2 replies; 5+ messages in thread
From: Haoran Wang @ 2026-10-16  6:57 UTC (permalink / raw)
  To: rostedt, mhiramat
  Cc: linux-kernel, linux-trace-kernel, mathieu.desnoyers,
	haoranwangsec

From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Haoran Wang <haoranwangsec@gmail.com>
Date: Fri, 16 Oct 2026 15:10:00 +0800
Subject: [PATCH] tracing: reject trace events with an empty TP_STRUCT__entry

trace_event_get_offsets() unconditionally dereferences the first entry of
an event class's fields list through list_first_entry().  A tracepoint
that declares an empty TP_STRUCT__entry never populates that list. Its
fields_array holds only the terminating sentinel, so list_first_entry()
returns the list head itself and the following tail->offset + tail->size
reads memory past struct trace_event_class.  The corrupted result is
then used as the BPF context-size bound in __perf_event_set_bpf_prog(),
which lets context-reading BPF programs attach to a zero-field tracepoint
when they should be rejected with -EACCES.

Reject such events at registration to fix this bug. event_init() is the
universal chokepoint invoked for both early-boot and module events, and
its return is enforced by __register_event(), which refuses to add the call
to ftrace_events.  An event whose fields_array[0].type is NULL (the
empty-TP_STRUCT__entry sentinel) is rejected with -EINVAL before
trace_event_get_offsets() can be reached.

Signed-off-by: Haoran Wang <haoranwangsec@gmail.com>
---
 kernel/trace/trace_events.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
--- a/kernel/trace/trace_events.c
+++ b/kernel/trace/trace_events.c
@@ -3279,6 +3279,10 @@ static int event_init(struct trace_event_call *call)
 	if (WARN_ON(!name))
 		return -EINVAL;
 
+	if (call->class->fields_array &&
+	    !call->class->fields_array[0].type)
+		return -EINVAL;
+
 	if (call->class->raw_init) {
 		ret = call->class->raw_init(call);
 		if (ret < 0 && ret != -ENOSYS)
-- 
2.43.0

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

* Re: [PATCH] tracing: reject trace events with an empty TP_STRUCT__entry
  2026-10-16  6:57 [PATCH] tracing: reject trace events with an empty TP_STRUCT__entry Haoran Wang
@ 2026-10-16  7:08 ` sashiko-bot
  2026-10-16 14:13 ` Steven Rostedt
  1 sibling, 0 replies; 5+ messages in thread
From: sashiko-bot @ 2026-10-16  7:08 UTC (permalink / raw)
  To: Haoran Wang; +Cc: linux-trace-kernel

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] tracing: user_events broken by fields_array[0].type check
--

commit e8b9d44518a8f72216cba6b41f5a36c08fdcef3c
Author: Haoran Wang <haoranwangsec@gmail.com>

tracing: reject trace events with an empty TP_STRUCT__entry

This patch adds a check in event_init() to reject trace events that have
an empty TP_STRUCT__entry by returning -EINVAL if the first element of
fields_array has a NULL type.

> diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
> index 30c0ddf90887f..9f79fbdf40a20 100644
> --- a/kernel/trace/trace_events.c
> +++ b/kernel/trace/trace_events.c
> @@ -3406,6 +3406,10 @@ static int event_init(struct trace_event_call *call)
>  	if (WARN_ON(!name))
>  		return -EINVAL;
>  
> +	if (call->class->fields_array &&
> +	    !call->class->fields_array[0].type)
> +		return -EINVAL;

[Severity: High]
Does this inadvertently break the user_events subsystem?

In kernel/trace/trace_events_user.c, user_events are assigned a statically
initialized dummy array:

kernel/trace/trace_events_user.c:
static struct trace_event_fields user_event_fields_array[1];

Because this array is size 1 and zero-initialized, its first element's type
field is implicitly NULL. When user_events are registered from userspace via
the ABI, __register_event() calls event_init(), which will evaluate this new
check to true and unconditionally return -EINVAL.

Can we find another way to handle empty TP_STRUCT__entry events that won't
cause a feature regression for user_events?

> +
>  	if (call->class->raw_init) {
>  		ret = call->class->raw_init(call);
>  		if (ret < 0 && ret != -ENOSYS)

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/6ad1cad5.856c1b21.18f9b7.a2c3@mx.google.com?part=1

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

* [PATCH] tracing: reject trace events with an empty TP_STRUCT__entry
@ 2026-10-16  7:38 Haoran Wang
  2026-10-16  7:56 ` sashiko-bot
  0 siblings, 1 reply; 5+ messages in thread
From: Haoran Wang @ 2026-10-16  7:38 UTC (permalink / raw)
  To: rostedt, mhiramat
  Cc: linux-kernel, linux-trace-kernel, mathieu.desnoyers,
	haoranwangsec

From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Haoran Wang <haoranwangsec@gmail.com>
Date: Fri, 16 Oct 2026 15:10:00 +0800
Subject: [PATCH] tracing: reject trace events with an empty TP_STRUCT__entry

trace_event_get_offsets() unconditionally dereferences the first entry of
an event class's fields list through list_first_entry().  A tracepoint
that declares an empty TP_STRUCT__entry never populates that list. Its
fields_array holds only the terminating sentinel, so list_first_entry()
returns the list head itself and the following tail->offset + tail->size
reads memory past struct trace_event_class.  The corrupted result is
then used as the BPF context-size bound in __perf_event_set_bpf_prog(),
which lets context-reading BPF programs attach to a zero-field tracepoint
when they should be rejected with -EACCES.

Reject such events at registration can fix this bug. event_init() is the
universal chokepoint invoked for both early-boot and module events, and
its return is enforced by __register_event(), which refuses to add the call
to ftrace_events.  An event whose fields_array[0].type is NULL (the
empty-TP_STRUCT__entry sentinel) is rejected with -EINVAL before
trace_event_get_offsets() can be reached.  Dynamic events, which carry
the TRACE_FUNCTION_TYPE marker in fields_array[0].type and populate the
fields list lazily through a define_fields callback, are unaffected.

Signed-off-by: Haoran Wang <haoranwangsec@gmail.com>
---
 kernel/trace/trace_events.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
--- a/kernel/trace/trace_events.c
+++ b/kernel/trace/trace_events.c
@@ -3279,6 +3279,11 @@ static int event_init(struct trace_event_call *call)
 	if (WARN_ON(!name))
 		return -EINVAL;
 
+	if (call->class->fields_array &&
+	    !call->class->fields_array[0].type &&
+	    !call->class->get_fields)
+		return -EINVAL;
+
 	if (call->class->raw_init) {
 		ret = call->class->raw_init(call);
 		if (ret < 0 && ret != -ENOSYS)
-- 
2.43.0

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

* Re: [PATCH] tracing: reject trace events with an empty TP_STRUCT__entry
  2026-10-16  7:38 Haoran Wang
@ 2026-10-16  7:56 ` sashiko-bot
  0 siblings, 0 replies; 5+ messages in thread
From: sashiko-bot @ 2026-10-16  7:56 UTC (permalink / raw)
  To: Haoran Wang; +Cc: linux-trace-kernel

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] tracing: incomplete fix leaves BPF OOB read exploitable for zero-field user_events
--

commit 4bc63c3e7a59307c8dcbd3d4bfc283a9c62599d9
Author: Haoran Wang <haoranwangsec@gmail.com>

tracing: reject trace events with an empty TP_STRUCT__entry

This patch modifies event_init() to reject trace events with an empty
TP_STRUCT__entry by checking if the fields_array is empty. The fix attempts
to prevent an out-of-bounds read in trace_event_get_offsets() when BPF
programs attach to zero-field tracepoints, but it specifically exempts
events that implement a get_fields callback.

> diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
> index 30c0ddf90887f..4bdd31dd24234 100644
> --- a/kernel/trace/trace_events.c
> +++ b/kernel/trace/trace_events.c
> @@ -3406,6 +3406,11 @@ static int event_init(struct trace_event_call *call)
>  	if (WARN_ON(!name))
>  		return -EINVAL;
>  
> +	if (call->class->fields_array &&
> +	    !call->class->fields_array[0].type &&
> +	    !call->class->get_fields)
> +		return -EINVAL;
[Severity: High]
Does this leave the out-of-bounds read exploitable for zero-field user_events?

The added check explicitly exempts events that define a get_fields callback.
The user_events subsystem defines user_event_get_fields and therefore bypasses
this new validation.

Since unprivileged users can dynamically create user_events with zero fields
(for example, via echo 'my_event' > user_events_data), user_event_parse_fields()
returns early:

kernel/trace/trace_events_user.c:user_event_parse_fields() {
    ...
    if (args == NULL)
        return 0;
    ...
}

This results in an empty user->fields list. Because user_events have the
TRACE_EVENT_FL_TRACEPOINT flag, BPF programs can attach to them.
__perf_event_set_bpf_prog() then calls the unpatched
trace_event_get_offsets(), which dereferences the empty list via
list_first_entry():

kernel/trace/trace_events.c:trace_event_get_offsets() {
    ...
    tail = list_first_entry(head, struct ftrace_event_field, link);
    return tail->offset + tail->size;
}

This reads out-of-bounds memory, bypassing BPF context size validation and
potentially allowing an unprivileged attacker to load a BPF program that
reads adjacent kernel memory during tracepoint execution.

Should there be an additional check in trace_event_get_offsets() or should
zero-field user_events be rejected entirely?

> +
>  	if (call->class->raw_init) {
>  		ret = call->class->raw_init(call);
>  		if (ret < 0 && ret != -ENOSYS)

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/6ad1d48d.a461c41f.282b8.7a97@mx.google.com?part=1

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

* Re: [PATCH] tracing: reject trace events with an empty TP_STRUCT__entry
  2026-10-16  6:57 [PATCH] tracing: reject trace events with an empty TP_STRUCT__entry Haoran Wang
  2026-10-16  7:08 ` sashiko-bot
@ 2026-10-16 14:13 ` Steven Rostedt
  1 sibling, 0 replies; 5+ messages in thread
From: Steven Rostedt @ 2026-10-16 14:13 UTC (permalink / raw)
  To: Haoran Wang; +Cc: mhiramat, linux-kernel, linux-trace-kernel, mathieu.desnoyers

On Fri, 16 Oct 2026 14:57:22 +0800
Haoran Wang <haoranwangsec@gmail.com> wrote:

> From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
> From: Haoran Wang <haoranwangsec@gmail.com>
> Date: Fri, 16 Oct 2026 15:10:00 +0800
> Subject: [PATCH] tracing: reject trace events with an empty TP_STRUCT__entry
> 
> trace_event_get_offsets() unconditionally dereferences the first entry of
> an event class's fields list through list_first_entry().  A tracepoint
> that declares an empty TP_STRUCT__entry never populates that list. Its
> fields_array holds only the terminating sentinel, so list_first_entry()
> returns the list head itself and the following tail->offset + tail->size
> reads memory past struct trace_event_class.  The corrupted result is
> then used as the BPF context-size bound in __perf_event_set_bpf_prog(),
> which lets context-reading BPF programs attach to a zero-field tracepoint
> when they should be rejected with -EACCES.
> 
> Reject such events at registration to fix this bug. event_init() is the
> universal chokepoint invoked for both early-boot and module events, and
> its return is enforced by __register_event(), which refuses to add the call
> to ftrace_events.  An event whose fields_array[0].type is NULL (the
> empty-TP_STRUCT__entry sentinel) is rejected with -EINVAL before
> trace_event_get_offsets() can be reached.
> 

BTW, what bug are you trying to solve? Just some theoretical bug you found
from a static analyzer or AI?

-- Steve

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

end of thread, other threads:[~2026-10-16 14:14 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-16  6:57 [PATCH] tracing: reject trace events with an empty TP_STRUCT__entry Haoran Wang
2026-10-16  7:08 ` sashiko-bot
2026-10-16 14:13 ` Steven Rostedt
  -- strict thread matches above, loose matches on Subject: below --
2026-10-16  7:38 Haoran Wang
2026-10-16  7:56 ` sashiko-bot

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.