[PATCH AUTOSEL 6.18] tracing: Avoid possible signed 64-bit truncation

[PATCH AUTOSEL 6.18] tracing: Avoid possible signed 64-bit truncation

[Sasha Levin]

From: Ian Rogers <irogers@google.com>

[ Upstream commit 00f13e28a9c3acd40f0551cde7e9d2d1a41585bf ]

64-bit truncation to 32-bit can result in the sign of the truncated
value changing. The cmp_mod_entry is used in bsearch and so the
truncation could result in an invalid search order. This would only
happen were the addresses more than 2GB apart and so unlikely, but
let's fix the potentially broken compare anyway.

Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Link: https://patch.msgid.link/20260108002625.333331-1-irogers@google.com
Signed-off-by: Ian Rogers <irogers@google.com>
Acked-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

The original buggy code was introduced in v6.15 and later. It's only
present in 6.15+ kernels.

### 8. SUMMARY OF ANALYSIS

**The Bug:**
- The `cmp_mod_entry()` function uses subtraction of two `unsigned long`
  values and returns the result as `int`
- On 64-bit systems, if addresses differ by more than 2^31 (~2GB), the
  truncation from 64-bit to 32-bit can flip the sign
- This would cause `bsearch()` to make wrong decisions about search
  direction
- Result: potentially incorrect module address lookups in trace data

**The Fix:**
- Replaces arithmetic subtraction with simple comparisons
- Returns -1, 0, or 1 directly based on comparisons
- No overflow or truncation possible with the new code
- Logic is more readable and provably correct

**Stable Criteria Evaluation:**
1. ✅ **Obviously correct and tested**: Simple logic, reviewed by
   maintainers
2. ✅ **Fixes a real bug**: Yes, a potential comparator correctness bug
3. ⚠️ **Important issue**: Moderate - unlikely to trigger (requires >2GB
   address separation) but could cause incorrect trace output
4. ✅ **Small and contained**: Only changes one function body (~6 lines)
5. ✅ **No new features**: Pure bug fix
6. ✅ **Applies cleanly**: Should apply to 6.15+ kernels where this code
   exists

**Risk Assessment:**
- Very low risk - the change is small and the new logic is simpler
- The original code has a provable bug (integer overflow on truncation)
- The new code has no such issues

**Concerns:**
- The code only exists in 6.15+ kernels (introduced March 2025)
- The bug is "unlikely" per the author (requires addresses >2GB apart)
- No known real-world reports of this actually causing issues

### DECISION

This is a valid bug fix that:
- Fixes a real (though unlikely to trigger) bug in the comparator
  function
- Is very small and self-contained
- Has been reviewed and acked by the tracing maintainers
- Has near-zero regression risk
- Applies to 6.15+ kernels only

The fix is surgical, obviously correct, and addresses a potential
correctness issue. While the bug is unlikely to trigger in practice
(addresses must be >2GB apart), it could cause silent data corruption in
trace output when it does trigger. The fix is trivial and risk-free.

**YES**

 kernel/trace/trace.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 142e3b737f0bc..907923d5f8bbb 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -6061,10 +6061,10 @@ static int cmp_mod_entry(const void *key, const void *pivot)
 	unsigned long addr = (unsigned long)key;
 	const struct trace_mod_entry *ent = pivot;
 
-	if (addr >= ent[0].mod_addr && addr < ent[1].mod_addr)
-		return 0;
-	else
-		return addr - ent->mod_addr;
+	if (addr < ent[0].mod_addr)
+		return -1;
+
+	return addr >= ent[1].mod_addr;
 }
 
 /**
-- 
2.51.0

Re: [PATCH AUTOSEL 6.18] tracing: Avoid possible signed 64-bit truncation

[Steven Rostedt]

On Wed, 28 Jan 2026 17:33:16 -0500
Sasha Levin <sashal@kernel.org> wrote:

> LLM Generated explanations, may be completely bogus:
> 
> The original buggy code was introduced in v6.15 and later. It's only
> present in 6.15+ kernels.

Interesting self discussion from the LLM.

> 
> ### 8. SUMMARY OF ANALYSIS
> 
> **The Bug:**
> - The `cmp_mod_entry()` function uses subtraction of two `unsigned long`
>   values and returns the result as `int`
> - On 64-bit systems, if addresses differ by more than 2^31 (~2GB), the
>   truncation from 64-bit to 32-bit can flip the sign
> - This would cause `bsearch()` to make wrong decisions about search
>   direction
> - Result: potentially incorrect module address lookups in trace data
> 
> **The Fix:**
> - Replaces arithmetic subtraction with simple comparisons
> - Returns -1, 0, or 1 directly based on comparisons
> - No overflow or truncation possible with the new code
> - Logic is more readable and provably correct
> 
> **Stable Criteria Evaluation:**
> 1. ✅ **Obviously correct and tested**: Simple logic, reviewed by
>    maintainers
> 2. ✅ **Fixes a real bug**: Yes, a potential comparator correctness bug
> 3. ⚠️ **Important issue**: Moderate - unlikely to trigger (requires >2GB
>    address separation) but could cause incorrect trace output

I purposely didn't add Cc: stable because there's basically no chance of
two modules being more than 2GB apart.

> 4. ✅ **Small and contained**: Only changes one function body (~6 lines)
> 5. ✅ **No new features**: Pure bug fix
> 6. ✅ **Applies cleanly**: Should apply to 6.15+ kernels where this code
>    exists
> 
> **Risk Assessment:**
> - Very low risk - the change is small and the new logic is simpler
> - The original code has a provable bug (integer overflow on truncation)
> - The new code has no such issues
> 
> **Concerns:**
> - The code only exists in 6.15+ kernels (introduced March 2025)
> - The bug is "unlikely" per the author (requires addresses >2GB apart)
> - No known real-world reports of this actually causing issues
> 
> ### DECISION
> 
> This is a valid bug fix that:
> - Fixes a real (though unlikely to trigger) bug in the comparator
>   function
> - Is very small and self-contained
> - Has been reviewed and acked by the tracing maintainers
> - Has near-zero regression risk
> - Applies to 6.15+ kernels only
> 
> The fix is surgical, obviously correct, and addresses a potential
> correctness issue. While the bug is unlikely to trigger in practice
> (addresses must be >2GB apart), it could cause silent data corruption in
> trace output when it does trigger. The fix is trivial and risk-free.
> 
> **YES**

But I see the bot considered the same thing, but because of the risk factor
it decided to take it.

I have no issue with backporting it (I did label it a "fix"). I was
actually wondering if the bot would notice likeliness of it not happening.
It did, but decided to take it for "correctness" reasons vs risk factor.

Nice,

-- Steve

[PATCH AUTOSEL 6.18] tracing: Avoid possible signed 64-bit truncation

[Sasha Levin]

From: Ian Rogers <irogers@google.com>

[ Upstream commit 00f13e28a9c3acd40f0551cde7e9d2d1a41585bf ]

64-bit truncation to 32-bit can result in the sign of the truncated
value changing. The cmp_mod_entry is used in bsearch and so the
truncation could result in an invalid search order. This would only
happen were the addresses more than 2GB apart and so unlikely, but
let's fix the potentially broken compare anyway.

Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Link: https://patch.msgid.link/20260108002625.333331-1-irogers@google.com
Signed-off-by: Ian Rogers <irogers@google.com>
Acked-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

The original buggy code was introduced in v6.15 and later. It's only
present in 6.15+ kernels.

### 8. SUMMARY OF ANALYSIS

**The Bug:**
- The `cmp_mod_entry()` function uses subtraction of two `unsigned long`
  values and returns the result as `int`
- On 64-bit systems, if addresses differ by more than 2^31 (~2GB), the
  truncation from 64-bit to 32-bit can flip the sign
- This would cause `bsearch()` to make wrong decisions about search
  direction
- Result: potentially incorrect module address lookups in trace data

**The Fix:**
- Replaces arithmetic subtraction with simple comparisons
- Returns -1, 0, or 1 directly based on comparisons
- No overflow or truncation possible with the new code
- Logic is more readable and provably correct

**Stable Criteria Evaluation:**
1. ✅ **Obviously correct and tested**: Simple logic, reviewed by
   maintainers
2. ✅ **Fixes a real bug**: Yes, a potential comparator correctness bug
3. ⚠️ **Important issue**: Moderate - unlikely to trigger (requires >2GB
   address separation) but could cause incorrect trace output
4. ✅ **Small and contained**: Only changes one function body (~6 lines)
5. ✅ **No new features**: Pure bug fix
6. ✅ **Applies cleanly**: Should apply to 6.15+ kernels where this code
   exists

**Risk Assessment:**
- Very low risk - the change is small and the new logic is simpler
- The original code has a provable bug (integer overflow on truncation)
- The new code has no such issues

**Concerns:**
- The code only exists in 6.15+ kernels (introduced March 2025)
- The bug is "unlikely" per the author (requires addresses >2GB apart)
- No known real-world reports of this actually causing issues

### DECISION

This is a valid bug fix that:
- Fixes a real (though unlikely to trigger) bug in the comparator
  function
- Is very small and self-contained
- Has been reviewed and acked by the tracing maintainers
- Has near-zero regression risk
- Applies to 6.15+ kernels only

The fix is surgical, obviously correct, and addresses a potential
correctness issue. While the bug is unlikely to trigger in practice
(addresses must be >2GB apart), it could cause silent data corruption in
trace output when it does trigger. The fix is trivial and risk-free.

**YES**

 kernel/trace/trace.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 142e3b737f0bc..907923d5f8bbb 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -6061,10 +6061,10 @@ static int cmp_mod_entry(const void *key, const void *pivot)
 	unsigned long addr = (unsigned long)key;
 	const struct trace_mod_entry *ent = pivot;
 
-	if (addr >= ent[0].mod_addr && addr < ent[1].mod_addr)
-		return 0;
-	else
-		return addr - ent->mod_addr;
+	if (addr < ent[0].mod_addr)
+		return -1;
+
+	return addr >= ent[1].mod_addr;
 }
 
 /**
-- 
2.51.0