From: Shung-Hsi Yu shung-hsi.yu@suse.com
[ Upstream commit 53ebef53a657d7957d35dc2b953db64f1bb28065 ]
The calculation of the index used to access the mask field in 'struct bpf_raw_tp_null_args' is done with 'int' type, which could overflow when the tracepoint being attached has more than 8 arguments.
While none of the tracepoints mentioned in raw_tp_null_args[] currently have more than 8 arguments, there do exist tracepoints that had more than 8 arguments (e.g. iocost_iocg_forgive_debt), so use the correct type for calculation and avoid Smatch static checker warning.
Reported-by: Dan Carpenter dan.carpenter@linaro.org Signed-off-by: Shung-Hsi Yu shung-hsi.yu@suse.com Signed-off-by: Andrii Nakryiko andrii@kernel.org Acked-by: Kumar Kartikeya Dwivedi memxor@gmail.com Link: https://lore.kernel.org/bpf/20250418074946.35569-1-shung-hsi.yu@suse.com
Closes: https://lore.kernel.org/r/843a3b94-d53d-42db-93d4-be10a4090146@stanley.mount... Signed-off-by: Sasha Levin sashal@kernel.org ---
Now I have enough context to analyze this commit: **Analysis:** **YES** - This commit should be backported to stable kernel trees. **Extensive Explanation:** ## Issue Description The commit fixes a potential integer overflow in the BPF BTF subsystem. The bug is in lines 6830 and 6833 of `kernel/bpf/btf.c` where the calculation `(arg capability_test capability_test.c f2fs_folio_analysis.md ipv4_multipath_analysis.md ipv6_route_allocation_rcu_analysis.md ixgbe_e610_set_phys_id_analysis.md linux lpfc_timeout_analysis.md rtl_bb_delay_analysis.md rtw89_mlo_analysis.md tcp_multipath_load_balance_analysis.md type_size_check type_size_check.c veth_driver_analysis.md 4)` is done with `int` type when `arg` can theoretically be large enough to cause overflow. ## Code Analysis 1. **Variable Types**: - `arg` is declared as `u32` (from `btf_ctx_arg_idx` return type) - The shift operation was using `int` arithmetic: `(0x1 << (arg capability_test capability_test.c f2fs_folio_analysis.md ipv4_multipath_analysis.md ipv6_route_allocation_rcu_analysis.md ixgbe_e610_set_phys_id_analysis.md linux lpfc_timeout_analysis.md rtl_bb_delay_analysis.md rtw89_mlo_analysis.md tcp_multipath_load_balance_analysis.md type_size_check type_size_check.c veth_driver_analysis.md 4))` - The fix changes it to `unsigned long long`: `(0x1ULL << (arg capability_test capability_test.c f2fs_folio_analysis.md ipv4_multipath_analysis.md ipv6_route_allocation_rcu_analysis.md ixgbe_e610_set_phys_id_analysis.md linux lpfc_timeout_analysis.md rtl_bb_delay_analysis.md rtw89_mlo_analysis.md tcp_multipath_load_balance_analysis.md type_size_check type_size_check.c veth_driver_analysis.md 4))` 2. **The Overflow Scenario**: - When `arg >= 8`, the expression `(arg capability_test capability_test.c f2fs_folio_analysis.md ipv4_multipath_analysis.md ipv6_route_allocation_rcu_analysis.md ixgbe_e610_set_phys_id_analysis.md linux lpfc_timeout_analysis.md rtl_bb_delay_analysis.md rtw89_mlo_analysis.md tcp_multipath_load_balance_analysis.md type_size_check type_size_check.c veth_driver_analysis.md 4)` could be >= 32 - Shifting by >= 32 bits on a 32-bit `int` causes undefined behavior - Even on 64-bit systems, large values could overflow the intermediate calculation 3. **Specific Changes**: ```c // Before (vulnerable): if (raw_tp_null_args[i].mask & (0x1 << (arg capability_test capability_test.c f2fs_folio_analysis.md ipv4_multipath_analysis.md ipv6_route_allocation_rcu_analysis.md ixgbe_e610_set_phys_id_analysis.md linux lpfc_timeout_analysis.md rtl_bb_delay_analysis.md rtw89_mlo_analysis.md tcp_multipath_load_balance_analysis.md type_size_check type_size_check.c veth_driver_analysis.md 4))) if (raw_tp_null_args[i].mask & (0x2 << (arg capability_test capability_test.c f2fs_folio_analysis.md ipv4_multipath_analysis.md ipv6_route_allocation_rcu_analysis.md ixgbe_e610_set_phys_id_analysis.md linux lpfc_timeout_analysis.md rtl_bb_delay_analysis.md rtw89_mlo_analysis.md tcp_multipath_load_balance_analysis.md type_size_check type_size_check.c veth_driver_analysis.md 4))) // After (fixed): if (raw_tp_null_args[i].mask & (0x1ULL << (arg capability_test capability_test.c f2fs_folio_analysis.md ipv4_multipath_analysis.md ipv6_route_allocation_rcu_analysis.md ixgbe_e610_set_phys_id_analysis.md linux lpfc_timeout_analysis.md rtl_bb_delay_analysis.md rtw89_mlo_analysis.md tcp_multipath_load_balance_analysis.md type_size_check type_size_check.c veth_driver_analysis.md 4))) if (raw_tp_null_args[i].mask & (0x2ULL << (arg capability_test capability_test.c f2fs_folio_analysis.md ipv4_multipath_analysis.md ipv6_route_allocation_rcu_analysis.md ixgbe_e610_set_phys_id_analysis.md linux lpfc_timeout_analysis.md rtl_bb_delay_analysis.md rtw89_mlo_analysis.md tcp_multipath_load_balance_analysis.md type_size_check type_size_check.c veth_driver_analysis.md 4))) ``` ## Why This Should Be Backported 1. **Bug Fix**: This is a clear bug fix addressing undefined behavior that could cause incorrect BPF program verification. 2. **Small and Contained**: The fix is minimal - only changes two lines by adding `ULL` suffix to ensure 64-bit arithmetic. 3. **No Side Effects**: The change maintains the same logic but uses proper types, eliminating undefined behavior without changing functionality. 4. **Critical Subsystem**: This affects BPF verification, which is security-critical. Incorrect verification could lead to security issues. 5. **Static Analysis Detection**: The commit was triggered by a Smatch static checker warning, indicating this is a real issue that static analysis tools flag. 6. **Potential Real-World Impact**: While the commit mentions no current tracepoints have >8 arguments, the code path could be triggered by future tracepoints or module tracepoints, making this a defensive fix. 7. **Follows Stable Criteria**: - Important bugfix ✓ - Minimal risk ✓ - No architectural changes ✓ - Confined to BPF subsystem ✓ ## Comparison with Similar Commits Looking at the historical examples, this commit is most similar to "Similar Commit #5" which was marked as "Backport Status: YES" - it's a small type safety fix in BPF BTF code that prevents undefined behavior. The fix prevents a subtle but potentially serious issue in BPF program verification, making it an excellent candidate for stable backporting.
kernel/bpf/btf.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c index c3223e0db2f51..9b1c18a54918f 100644 --- a/kernel/bpf/btf.c +++ b/kernel/bpf/btf.c @@ -6708,10 +6708,10 @@ bool btf_ctx_access(int off, int size, enum bpf_access_type type, /* Is this a func with potential NULL args? */ if (strcmp(tname, raw_tp_null_args[i].func)) continue; - if (raw_tp_null_args[i].mask & (0x1 << (arg * 4))) + if (raw_tp_null_args[i].mask & (0x1ULL << (arg * 4))) info->reg_type |= PTR_MAYBE_NULL; /* Is the current arg IS_ERR? */ - if (raw_tp_null_args[i].mask & (0x2 << (arg * 4))) + if (raw_tp_null_args[i].mask & (0x2ULL << (arg * 4))) ptr_err_raw_tp = true; break; }