* [PATCH 7.2.y 2/3] tracing: Bound histogram expression strings with seq_buf
2026-10-22 12:54 ` [PATCH 7.2.y 1/3] tracing: Use __free() for expr_str() buffer Sasha Levin
@ 2026-10-22 12:54 ` Sasha Levin
2026-10-22 12:54 ` [PATCH 7.2.y 3/3] tracing: Fix NULL dereference when copying keys for a field variable Sasha Levin
1 sibling, 0 replies; 4+ messages in thread
From: Sasha Levin @ 2026-10-22 12:54 UTC (permalink / raw)
To: stable; +Cc: Pengpeng Hou, Steven Rostedt, Sasha Levin
From: Pengpeng Hou <pengpeng@iscas.ac.cn>
[ Upstream commit 935601a4276f28105498609f7f98aa66e86e9c96 ]
expr_str() allocates a fixed MAX_FILTER_STR_VAL buffer and then builds
expression names with a series of raw strcat() appends. Nested operands,
constants, field flags, and generated field names can push the rendered
string past that fixed limit before the name is attached to the hist
field.
Build expression strings with seq_buf and return -E2BIG when the
rendered name would exceed MAX_FILTER_STR_VAL. This keeps the existing
tracing-side limit while replacing the raw append logic with bounded
construction.
Link: https://patch.msgid.link/20260611055945.22348-4-pengpeng@iscas.ac.cn
Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
Stable-dep-of: a0777b9e5315 ("tracing: Fix NULL dereference when copying keys for a field variable")
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
kernel/trace/trace_events_hist.c | 57 ++++++++++++++++++--------------
1 file changed, 33 insertions(+), 24 deletions(-)
diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c
index 81bb1cf96c751..8caeb6bfb7d5e 100644
--- a/kernel/trace/trace_events_hist.c
+++ b/kernel/trace/trace_events_hist.c
@@ -94,7 +94,6 @@ typedef u64 (*hist_field_fn_t) (struct hist_field *field,
#define HIST_FIELD_OPERANDS_MAX 2
#define HIST_FIELDS_MAX (TRACING_MAP_FIELDS_MAX + TRACING_MAP_VARS_MAX)
#define HIST_ACTIONS_MAX 8
-#define HIST_CONST_DIGITS_MAX 21
#define HIST_DIV_SHIFT 20 /* For optimizing division by constants */
enum field_op_id {
@@ -1732,33 +1731,36 @@ static const char *get_hist_field_flags(struct hist_field *hist_field)
return flags_str;
}
-static void expr_field_str(struct hist_field *field, char *expr)
+static bool expr_field_str(struct hist_field *field, struct seq_buf *s)
{
+ const char *field_name;
+
if (field->flags & HIST_FIELD_FL_VAR_REF) {
if (!field->system)
- strcat(expr, "$");
- } else if (field->flags & HIST_FIELD_FL_CONST) {
- char str[HIST_CONST_DIGITS_MAX];
+ seq_buf_putc(s, '$');
+ } else if (field->flags & HIST_FIELD_FL_CONST)
+ seq_buf_printf(s, "%llu", field->constant);
- snprintf(str, HIST_CONST_DIGITS_MAX, "%llu", field->constant);
- strcat(expr, str);
- }
+ field_name = hist_field_name(field, 0);
+ if (!field_name)
+ return false;
- strcat(expr, hist_field_name(field, 0));
+ seq_buf_puts(s, field_name);
if (field->flags && !(field->flags & HIST_FIELD_FL_VAR_REF)) {
const char *flags_str = get_hist_field_flags(field);
- if (flags_str) {
- strcat(expr, ".");
- strcat(expr, flags_str);
- }
+ if (flags_str)
+ seq_buf_printf(s, ".%s", flags_str);
}
+
+ return !seq_buf_has_overflowed(s);
}
static char *expr_str(struct hist_field *field, unsigned int level)
{
char *expr __free(kfree) = NULL;
+ struct seq_buf s;
if (level > 1)
return NULL;
@@ -1767,47 +1769,54 @@ static char *expr_str(struct hist_field *field, unsigned int level)
if (!expr)
return NULL;
+ seq_buf_init(&s, expr, MAX_FILTER_STR_VAL);
+
if (!field->operands[0]) {
- expr_field_str(field, expr);
+ if (!expr_field_str(field, &s))
+ return ERR_PTR(-E2BIG);
+
return_ptr(expr);
}
if (field->operator == FIELD_OP_UNARY_MINUS) {
char *subexpr;
- strcat(expr, "-(");
subexpr = expr_str(field->operands[0], ++level);
if (!subexpr)
return NULL;
- strcat(expr, subexpr);
- strcat(expr, ")");
-
+ seq_buf_printf(&s, "-(%s)", subexpr);
kfree(subexpr);
+ if (seq_buf_has_overflowed(&s))
+ return ERR_PTR(-E2BIG);
+
return_ptr(expr);
}
- expr_field_str(field->operands[0], expr);
+ if (!expr_field_str(field->operands[0], &s))
+ return ERR_PTR(-E2BIG);
switch (field->operator) {
case FIELD_OP_MINUS:
- strcat(expr, "-");
+ seq_buf_putc(&s, '-');
break;
case FIELD_OP_PLUS:
- strcat(expr, "+");
+ seq_buf_putc(&s, '+');
break;
case FIELD_OP_DIV:
- strcat(expr, "/");
+ seq_buf_putc(&s, '/');
break;
case FIELD_OP_MULT:
- strcat(expr, "*");
+ seq_buf_putc(&s, '*');
break;
default:
return NULL;
}
- expr_field_str(field->operands[1], expr);
+ if (seq_buf_has_overflowed(&s) ||
+ !expr_field_str(field->operands[1], &s))
+ return ERR_PTR(-E2BIG);
return_ptr(expr);
}
--
2.53.0
^ permalink raw reply related [flat|nested] 4+ messages in thread* [PATCH 7.2.y 3/3] tracing: Fix NULL dereference when copying keys for a field variable
2026-10-22 12:54 ` [PATCH 7.2.y 1/3] tracing: Use __free() for expr_str() buffer Sasha Levin
2026-10-22 12:54 ` [PATCH 7.2.y 2/3] tracing: Bound histogram expression strings with seq_buf Sasha Levin
@ 2026-10-22 12:54 ` Sasha Levin
1 sibling, 0 replies; 4+ messages in thread
From: Sasha Levin @ 2026-10-22 12:54 UTC (permalink / raw)
To: stable; +Cc: Donggeun Yoo, Steven Rostedt, Sasha Levin
From: Donggeun Yoo <donggeunyoo.kernel@gmail.com>
[ Upstream commit a0777b9e53157f1f6848a0fd0650f5465391ea81 ]
In event_hist_trigger_parse() where it needs to create actions like
"onmatch", it calls:
event_hist_trigger_parse() {
create_actions() {
action_create() {
trace_action_create() {
trace_action_create_field_var() {
create_field_var_hist()
Where create_field_var_hist() does a loop on the hist_data
representing the keys. The issue is, if the keys uses one of the
pseudo field types (like common_cpu), the hist_data field element
will have NULL for its field member causing a NULL pointer
dereference when accessing the key_field->field->name.
# echo 'hist:keys=common_cpu:ts0=common_timestamp.usecs' > \
events/sched/sched_waking/trigger
# echo 'my_synth u64 lat; int prio' > synthetic_events
# echo 'hist:keys=common_cpu:wakeup_lat=common_timestamp.usecs-$ts0:\
onmatch(sched.sched_waking).my_synth($wakeup_lat,prio)' > \
events/sched/sched_switch/trigger
KASAN: null-ptr-deref in range [0x0000000000000010-0x0000000000000017]
RIP: 0010:create_field_var_hist+0x771/0x1380
Call Trace:
trace_action_create_field_var+0x360/0x600
trace_action_create+0x393/0xe80
event_hist_trigger_parse+0x3e47/0x69e0
trigger_process_regex+0x1a6/0x250
event_trigger_write+0xce/0x160
Instead of accessing it directly, use the proper handler
expr_field_str() to get the name.
The three previous patches are prerequisites: without them a .buckets
key and a common_stacktrace key each render into a command that cannot
be parsed back.
Cc: stable@vger.kernel.org
Link: https://patch.msgid.link/20260914053443.981201-5-donggeunyoo.kernel@gmail.com
Fixes: 02205a6752f2 ("tracing: Add support for 'field variables'")
Signed-off-by: Donggeun Yoo <donggeunyoo.kernel@gmail.com>
Assisted-by: Claude:claude-fable-5
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
kernel/trace/trace_events_hist.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c
index 8caeb6bfb7d5e..2295ca7bccefd 100644
--- a/kernel/trace/trace_events_hist.c
+++ b/kernel/trace/trace_events_hist.c
@@ -3078,7 +3078,7 @@ create_field_var_hist(struct hist_trigger_data *target_hist_data,
key_field = hist_data->fields[i];
if (!first)
seq_buf_putc(&s, ',');
- seq_buf_puts(&s, key_field->field->name);
+ expr_field_str(key_field, &s);
first = false;
}
--
2.53.0
^ permalink raw reply related [flat|nested] 4+ messages in thread