* [PATCH AUTOSEL 6.15 070/118] tracing: Only return an adjusted address if it matches the kernel address
[not found] <20250604005049.4147522-1-sashal@kernel.org>
@ 2025-06-04 0:50 ` Sasha Levin
2025-06-04 1:15 ` Steven Rostedt
0 siblings, 1 reply; 2+ messages in thread
From: Sasha Levin @ 2025-06-04 0:50 UTC (permalink / raw)
To: patches, stable
Cc: Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers, Sasha Levin,
linux-kernel, linux-trace-kernel
From: Steven Rostedt <rostedt@goodmis.org>
[ Upstream commit 00d872dd541cdf22230510201a1baf58f0147db9 ]
The trace_adjust_address() will take a given address and examine the
persistent ring buffer to see if the address matches a module that is
listed there. If it does not, it will just adjust the value to the core
kernel delta. But if the address was for something that was not part of
the core kernel text or data it should not be adjusted.
Check the result of the adjustment and only return the adjustment if it
lands in the current kernel text or data. If not, return the original
address.
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Link: https://lore.kernel.org/20250506102300.0ba2f9e0@gandalf.local.home
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
**YES** This commit should be backported to stable kernel trees based on
the following comprehensive analysis: ## Security Analysis ### 1.
**Critical Security Fix** The commit addresses a significant security
vulnerability in the `trace_adjust_address()` function. The change on
lines 6148-6149 adds crucial validation: ```c raddr = addr +
tr->text_delta; return __is_kernel(raddr) || is_kernel_core_data(raddr)
|| is_kernel_rodata(raddr) ? raddr : addr; ``` **Before the fix**: The
function would blindly return `addr + tr->text_delta` without validating
whether the resulting address falls within legitimate kernel memory
regions. **After the fix**: The function validates that adjusted
addresses only point to valid kernel sections (text, core data, or read-
only data). If the adjusted address doesn't fall within these legitimate
regions, it returns the original address unchanged. ### 2. **KASLR
Protection** This fix is particularly important for KASLR (Kernel
Address Space Layout Randomization) security: - **Information Disclosure
Risk**: Without validation, the function could return addresses pointing
to arbitrary memory locations, potentially leaking kernel address layout
information - **KASLR Bypass**: Invalid address adjustments could reveal
the kernel's memory layout, defeating KASLR protections - **Cross-boot
attacks**: The persistent ring buffer functionality could be exploited
to extract address information from previous boot sessions ### 3.
**Memory Safety** The validation prevents potential memory corruption
scenarios: - **Out-of-bounds access**: Ensures addresses used for symbol
resolution and string dereferencing are within valid kernel regions -
**Kernel crashes**: Prevents dereferencing of invalid addresses that
could cause kernel panics - **Information leakage**: Stops potential
disclosure of arbitrary kernel memory contents ### 4. **Limited Risk of
Regression** This is a defensive fix that only affects the tracing
subsystem: - **Minimal impact**: Only changes behavior when addresses
would be invalid anyway - **Backwards compatible**: Legitimate addresses
continue to work as before - **Contained scope**: Limited to persistent
ring buffer functionality ### 5. **Alignment with Stable Tree Criteria**
The commit meets all the criteria for stable tree backporting: - ✅
**Important bug fix**: Addresses a security vulnerability - ✅ **Small
and contained**: Only adds validation logic, doesn't change architecture
- ✅ **Minimal regression risk**: Defensive change that only affects
invalid cases - ✅ **Clear benefit**: Improves kernel security without
breaking functionality - ✅ **Non-feature**: Pure security fix, not
adding new functionality ### 6. **Real-world Impact** This vulnerability
could be exploited in environments where: - Persistent tracing is
enabled across reboots - Attackers have access to trace output - Systems
rely on KASLR for security The fix ensures that the tracing subsystem
cannot be used as a vector for kernel address space information
disclosure or memory corruption attacks. **Conclusion**: This is a clear
security fix that should be backported to maintain the security
integrity of stable kernel releases.
kernel/trace/trace.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 5b8db27fb6ef3..01572ef79802f 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -6032,6 +6032,7 @@ unsigned long trace_adjust_address(struct trace_array *tr, unsigned long addr)
struct trace_module_delta *module_delta;
struct trace_scratch *tscratch;
struct trace_mod_entry *entry;
+ unsigned long raddr;
int idx = 0, nr_entries;
/* If we don't have last boot delta, return the address */
@@ -6045,7 +6046,9 @@ unsigned long trace_adjust_address(struct trace_array *tr, unsigned long addr)
module_delta = READ_ONCE(tr->module_delta);
if (!module_delta || !tscratch->nr_entries ||
tscratch->entries[0].mod_addr > addr) {
- return addr + tr->text_delta;
+ raddr = addr + tr->text_delta;
+ return __is_kernel(raddr) || is_kernel_core_data(raddr) ||
+ is_kernel_rodata(raddr) ? raddr : addr;
}
/* Note that entries must be sorted. */
--
2.39.5
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH AUTOSEL 6.15 070/118] tracing: Only return an adjusted address if it matches the kernel address
2025-06-04 0:50 ` [PATCH AUTOSEL 6.15 070/118] tracing: Only return an adjusted address if it matches the kernel address Sasha Levin
@ 2025-06-04 1:15 ` Steven Rostedt
0 siblings, 0 replies; 2+ messages in thread
From: Steven Rostedt @ 2025-06-04 1:15 UTC (permalink / raw)
To: Sasha Levin
Cc: patches, stable, Masami Hiramatsu, Mathieu Desnoyers,
linux-kernel, linux-trace-kernel
On Tue, 3 Jun 2025 20:50:01 -0400
Sasha Levin <sashal@kernel.org> wrote:
> From: Steven Rostedt <rostedt@goodmis.org>
>
> [ Upstream commit 00d872dd541cdf22230510201a1baf58f0147db9 ]
>
> The trace_adjust_address() will take a given address and examine the
> persistent ring buffer to see if the address matches a module that is
> listed there. If it does not, it will just adjust the value to the core
> kernel delta. But if the address was for something that was not part of
> the core kernel text or data it should not be adjusted.
>
> Check the result of the adjustment and only return the adjustment if it
> lands in the current kernel text or data. If not, return the original
> address.
>
> Cc: Masami Hiramatsu <mhiramat@kernel.org>
> Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
> Link: https://lore.kernel.org/20250506102300.0ba2f9e0@gandalf.local.home
> Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
> Signed-off-by: Sasha Levin <sashal@kernel.org>
> ---
>
I guess the following blurb is new.
> **YES** This commit should be backported to stable kernel trees based on
Hmm, I'm not so sure the analysis is correct.
> the following comprehensive analysis: ## Security Analysis ### 1.
> **Critical Security Fix** The commit addresses a significant security
> vulnerability in the `trace_adjust_address()` function. The change on
> lines 6148-6149 adds crucial validation: ```c raddr = addr +
> tr->text_delta; return __is_kernel(raddr) || is_kernel_core_data(raddr)
> || is_kernel_rodata(raddr) ? raddr : addr; ``` **Before the fix**: The
> function would blindly return `addr + tr->text_delta` without validating
> whether the resulting address falls within legitimate kernel memory
If you look at the code, it will return the address regardless if it is
within the kernel memory or not.
This is called when reading addresses that are in the persistent ring
buffer from a previous boot.
Before the "fix":
It would always add the text_delta to the address.
The issue without that is that it could be adjusting a pointer that was to
allocated memory. It makes no sense to do this. The reason for doing this
adjustment is because a lot of reads of addresses use "%pS", and we care
only about getting a proper kallsyms of the address.
Thus what is done is:
raddr = addr + tr->text_delta;
return __is_kernel(raddr) || is_kernel_core_data(raddr) ||
is_kernel_rodata(raddr) ? raddr : addr;
Which does the adjustment, and if it falls into kernel memory or data
return that adjustment, otherwise return the original address. The reason
is that by returning the adjusted memory, it may fall into a module that we
do not want to print kallsyms for.
> regions. **After the fix**: The function validates that adjusted
> addresses only point to valid kernel sections (text, core data, or read-
> only data). If the adjusted address doesn't fall within these legitimate
> regions, it returns the original address unchanged. ### 2. **KASLR
> Protection** This fix is particularly important for KASLR (Kernel
> Address Space Layout Randomization) security: - **Information Disclosure
It doesn't risk any KASLR information. All addresses used by
trace_adjust_address() is from a pointer that existed in a previous boot.
The adjustment is pretty meaningless if it's not in kernel text or data.
> Risk**: Without validation, the function could return addresses pointing
> to arbitrary memory locations, potentially leaking kernel address layout
> information - **KASLR Bypass**: Invalid address adjustments could reveal
> the kernel's memory layout, defeating KASLR protections - **Cross-boot
> attacks**: The persistent ring buffer functionality could be exploited
> to extract address information from previous boot sessions ### 3.
> **Memory Safety** The validation prevents potential memory corruption
> scenarios: - **Out-of-bounds access**: Ensures addresses used for symbol
> resolution and string dereferencing are within valid kernel regions -
> **Kernel crashes**: Prevents dereferencing of invalid addresses that
> could cause kernel panics - **Information leakage**: Stops potential
> disclosure of arbitrary kernel memory contents ### 4. **Limited Risk of
> Regression** This is a defensive fix that only affects the tracing
> subsystem: - **Minimal impact**: Only changes behavior when addresses
> would be invalid anyway - **Backwards compatible**: Legitimate addresses
> continue to work as before - **Contained scope**: Limited to persistent
> ring buffer functionality ### 5. **Alignment with Stable Tree Criteria**
> The commit meets all the criteria for stable tree backporting: - ✅
> **Important bug fix**: Addresses a security vulnerability - ✅ **Small
> and contained**: Only adds validation logic, doesn't change architecture
> - ✅ **Minimal regression risk**: Defensive change that only affects
> invalid cases - ✅ **Clear benefit**: Improves kernel security without
> breaking functionality - ✅ **Non-feature**: Pure security fix, not
> adding new functionality ### 6. **Real-world Impact** This vulnerability
> could be exploited in environments where: - Persistent tracing is
> enabled across reboots - Attackers have access to trace output - Systems
Yes persistent tracing is enabled across reboots and the address is from a
previous boot. It does return the actual address of the current boot to use
with %pS when it was on the kernel text or data address. When it isn't
(likely a module address) the adjustment is meaningless and may give bad
trace output at most.
If an attacker has access to trace output KASLR is already lost, as
function tracing records raw addresses and exposes everything KASLR, which
is why reading these files is a privilege operation.
I won't argue against backporting, but I just wanted to state this analysis
may not be correct.
-- Steve
> rely on KASLR for security The fix ensures that the tracing subsystem
> cannot be used as a vector for kernel address space information
> disclosure or memory corruption attacks. **Conclusion**: This is a clear
> security fix that should be backported to maintain the security
> integrity of stable kernel releases.
>
> kernel/trace/trace.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> index 5b8db27fb6ef3..01572ef79802f 100644
> --- a/kernel/trace/trace.c
> +++ b/kernel/trace/trace.c
> @@ -6032,6 +6032,7 @@ unsigned long trace_adjust_address(struct trace_array *tr, unsigned long addr)
> struct trace_module_delta *module_delta;
> struct trace_scratch *tscratch;
> struct trace_mod_entry *entry;
> + unsigned long raddr;
> int idx = 0, nr_entries;
>
> /* If we don't have last boot delta, return the address */
> @@ -6045,7 +6046,9 @@ unsigned long trace_adjust_address(struct trace_array *tr, unsigned long addr)
> module_delta = READ_ONCE(tr->module_delta);
> if (!module_delta || !tscratch->nr_entries ||
> tscratch->entries[0].mod_addr > addr) {
> - return addr + tr->text_delta;
> + raddr = addr + tr->text_delta;
> + return __is_kernel(raddr) || is_kernel_core_data(raddr) ||
> + is_kernel_rodata(raddr) ? raddr : addr;
> }
>
> /* Note that entries must be sorted. */
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2025-06-04 1:14 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20250604005049.4147522-1-sashal@kernel.org>
2025-06-04 0:50 ` [PATCH AUTOSEL 6.15 070/118] tracing: Only return an adjusted address if it matches the kernel address Sasha Levin
2025-06-04 1:15 ` Steven Rostedt
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox