| From 426e81177951679c34598b564743c9371e8ed265 Mon Sep 17 00:00:00 2001 | |
| From: Sasha Levin <sashal@kernel.org> | |
| Date: Thu, 10 Sep 2026 22:12:09 -0400 | |
| Subject: tracing: Take trace_array reference when opening a tracer options | |
| file | |
| From: Steven Rostedt <rostedt@goodmis.org> | |
| [ Upstream commit ed0aff60f83a9bdc2f6556376ac79c96b3ce7e80 ] | |
| When a tracer option file is opened, it is passed a descriptor that points | |
| to an element on the trace_array's topts array. This element has | |
| information to find the trace array and other information. It uses this | |
| element to take a reference of the trace_array so that the trace_array | |
| does not get removed while this file is opened. | |
| Unfortunately, there's a race condition where the element itself could be | |
| freed by the removal of the instance the trace_array represents causing a | |
| use-after-free as this element that is used to find the trace_array to | |
| increment its reference counter is also freed when the instance is | |
| removed. | |
| To solve this, add a trace_array_tracer_options_get() helper function that | |
| will take the address of the element that is passed to the open function | |
| by the inode->i_private pointer and search all the trace_arrays under a | |
| lock to find the one that the element's address is in the range of the | |
| trace_arrays topts array elements. When a match happens, that trace_array's | |
| reference would be increased. | |
| Note, there's a race where if an admin was deleting and creating trace | |
| instances at the same time and the memory of the old trace_array's array | |
| matched the memory of the new trace_array that it could in theory open the | |
| option from the wrong trace array. But we do not care because it would be | |
| stupid to perform that kind of action. As long as the only thing that can | |
| happen is that the option from the wrong trace array is used and doesn't | |
| crash the kernel it will only make the user confused. But if they are | |
| doing something stupid like this, they are already confused, so no harm | |
| done. | |
| Cc: stable@vger.kernel.org | |
| Link: https://patch.msgid.link/20260910221209.62dad8d3@robin | |
| Fixes: 7e2cfbd2d3c86 ("tracing: Have option files inc the trace array ref count") | |
| Reported-by: sashiko-bot@kernel.org | |
| Closes: https://lore.kernel.org/linux-trace-kernel/20260902121918.5a9e9d1b@gandalf.local.home/ | |
| Signed-off-by: Steven Rostedt <rostedt@goodmis.org> | |
| Signed-off-by: Sasha Levin <sashal@kernel.org> | |
| --- | |
| kernel/trace/trace.c | 48 +++++++++++++++++++++++++++++++++++++++++++- | |
| kernel/trace/trace.h | 1 + | |
| 2 files changed, 48 insertions(+), 1 deletion(-) | |
| diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c | |
| index 7f25ae6330942..0c0f31e63d05d 100644 | |
| --- a/kernel/trace/trace.c | |
| +++ b/kernel/trace/trace.c | |
| @@ -9279,12 +9279,57 @@ trace_options_write(struct file *filp, const char __user *ubuf, size_t cnt, | |
| return cnt; | |
| } | |
| +static bool tr_option_match(struct trace_array *tr, void *topt) | |
| +{ | |
| + for (int i = 0; i < tr->nr_topts; i++) { | |
| + struct trace_options *tr_topts = &tr->topts[i]; | |
| + | |
| + if (topt >= (void *)&tr_topts->topts[0] && | |
| + topt < (void *)&tr_topts->topts[tr_topts->nr_topts]) | |
| + return true; | |
| + } | |
| + return false; | |
| +} | |
| + | |
| +/* | |
| + * The topt is the address of a trace_array->topts[] element that holds the | |
| + * the tracer options descriptor. But since the trace_array reference has not | |
| + * been taken yet, it cannot be dereferenced as it could have been freed by | |
| + * a rmdir of the instance the trace_array represents. | |
| + * | |
| + * Search the list of trace_arrays and compare the topt to the address of | |
| + * the entire trace_array topts array for each trace_array in the list. | |
| + * If one is matched, then take the reference and return it. If not, the | |
| + * trace_array no longer exits. | |
| + */ | |
| +static int trace_array_tracer_options_get(void *topt) | |
| +{ | |
| + struct trace_array *tr; | |
| + int ret; | |
| + | |
| + ret = security_locked_down(LOCKDOWN_TRACEFS); | |
| + if (ret) | |
| + return ret; | |
| + | |
| + if (tracing_disabled) | |
| + return -ENODEV; | |
| + | |
| + guard(mutex)(&trace_types_lock); | |
| + list_for_each_entry(tr, &ftrace_trace_arrays, list) { | |
| + if (tr_option_match(tr, topt)) { | |
| + tr->ref++; | |
| + return 0; | |
| + } | |
| + } | |
| + return -ENODEV; | |
| +} | |
| + | |
| static int tracing_open_options(struct inode *inode, struct file *filp) | |
| { | |
| struct trace_option_dentry *topt = inode->i_private; | |
| int ret; | |
| - ret = tracing_check_open_get_tr(topt->tr); | |
| + ret = trace_array_tracer_options_get(topt); | |
| if (ret) | |
| return ret; | |
| @@ -9568,6 +9613,7 @@ create_trace_option_files(struct trace_array *tr, struct tracer *tracer) | |
| tr->topts = tr_topts; | |
| tr->topts[tr->nr_topts].tracer = tracer; | |
| tr->topts[tr->nr_topts].topts = topts; | |
| + tr->topts[tr->nr_topts].nr_topts = cnt; | |
| tr->nr_topts++; | |
| for (cnt = 0; opts[cnt].name; cnt++) { | |
| diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h | |
| index ab88b87ac2ee2..39c425a9538a5 100644 | |
| --- a/kernel/trace/trace.h | |
| +++ b/kernel/trace/trace.h | |
| @@ -224,6 +224,7 @@ struct array_buffer { | |
| struct trace_options { | |
| struct tracer *tracer; | |
| struct trace_option_dentry *topts; | |
| + int nr_topts; | |
| }; | |
| struct trace_pid_list *trace_pid_list_alloc(void); | |
| -- | |
| 2.53.0 | |