public inbox for bpf@vger.kernel.org
 help / color / mirror / Atom feed
From: Sasha Levin <sashal@kernel.org>
To: patches@lists.linux.dev, stable@vger.kernel.org
Cc: Chenghao Duan <duanchenghao@kylinos.cn>,
	Pu Lehui <pulehui@huawei.com>,
	Alexei Starovoitov <ast@kernel.org>,
	Sasha Levin <sashal@kernel.org>,
	daniel@iogearbox.net, andrii@kernel.org, bjorn@kernel.org,
	pjw@kernel.org, palmer@dabbelt.com, aou@eecs.berkeley.edu,
	bpf@vger.kernel.org, linux-riscv@lists.infradead.org
Subject: [PATCH AUTOSEL 6.17-6.6] riscv: bpf: Fix uninitialized symbol 'retval_off'
Date: Thu,  9 Oct 2025 11:56:16 -0400	[thread overview]
Message-ID: <20251009155752.773732-110-sashal@kernel.org> (raw)
In-Reply-To: <20251009155752.773732-1-sashal@kernel.org>

From: Chenghao Duan <duanchenghao@kylinos.cn>

[ Upstream commit d0bf7cd5df18466d969bb60e8890b74cf96081ca ]

In the __arch_prepare_bpf_trampoline() function, retval_off is only
meaningful when save_ret is true, so the current logic is correct.
However, in the original logic, retval_off is only initialized under
certain conditions; for example, in the fmod_ret logic, the compiler is
not aware that the flags of the fmod_ret program (prog) have set
BPF_TRAMP_F_CALL_ORIG, which results in an uninitialized symbol
compilation warning.

So initialize retval_off unconditionally to fix it.

Signed-off-by: Chenghao Duan <duanchenghao@kylinos.cn>
Reviewed-by: Pu Lehui <pulehui@huawei.com>
Link: https://lore.kernel.org/r/20250922062244.822937-2-duanchenghao@kylinos.cn
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

YES

- What changed: The patch moves `retval_off = stack_size;` out of the
  `if (save_ret)` block so it’s always initialized. Previously
  `retval_off` was only assigned when `save_ret` was true.
  - Before: `retval_off` assigned only inside `if (save_ret) { ... }`
  - After: `retval_off` assigned unconditionally immediately after the
    optional `stack_size += 16`
  - Reference: arch/riscv/net/bpf_jit_comp64.c:1066

- Why it matters: `retval_off` is used in code paths not explicitly
  guarded by `save_ret`, which makes compilers think it can be used
  uninitialized and emit a warning (e.g., -Wmaybe-uninitialized), even
  though, logically, those paths only occur with flags that imply
  `save_ret` must be true.
  - Unconditional uses in fmod_ret path:
    - Zeroing return value slot: arch/riscv/net/bpf_jit_comp64.c:1157
    - Loading return value: arch/riscv/net/bpf_jit_comp64.c:1163
  - Unconditional uses in call-orig path:
    - Store original function’s return in reserved slot:
      arch/riscv/net/bpf_jit_comp64.c:1176
    - Store BPF R0: arch/riscv/net/bpf_jit_comp64.c:1177
  - Final restore guarded by `save_ret`, confirming the semantic intent:
    arch/riscv/net/bpf_jit_comp64.c:1209

- Bug scope and user impact:
  - This is a build correctness fix that eliminates spurious “maybe-
    uninitialized” warnings that can be promoted to errors in some
    configurations or toolchains. It does not change runtime behavior
    because the only meaningful use of `retval_off` (e.g., restoring
    return values) is already guarded by `save_ret`. When `save_ret` is
    false, `retval_off`’s value is ignored by the logic that matters.
  - The warning can affect users building with stricter warning settings
    or newer compilers; resolving it improves build reliability for
    RISC-V with BPF trampolines.

- Containment and risk:
  - The change is tiny and contained to a single file/function in the
    RISC-V BPF JIT trampoline.
  - No new features, APIs, or architectural changes; no functional logic
    changed for valid flag combinations.
  - Safe even if misused flags were ever passed: `retval_off` now has a
    defined value, avoiding UB from uninitialized use.

- Applicability to stable trees:
  - The affected pattern exists in stable series that have the RISC-V
    BPF trampoline (e.g., v6.6 shows the same conditional
    initialization, with unconditional uses later). See v6.6 code where
    `retval_off` is only set under `if (save_ret)` and is used in the
    fmod_ret block and call-orig sequence without an explicit `save_ret`
    guard, mirroring the warning scenario.
  - Mainline commit: d0bf7cd5df184 (“riscv: bpf: Fix uninitialized
    symbol 'retval_off'”).
  - Likely Fixes: 25ad10658dc10 (“riscv, bpf: Adapt bpf trampoline to
    optimized riscv ftrace framework”), which introduced the trampoline
    structure that uses `retval_off` this way.

- Stable criteria check:
  - Fixes a real build issue (warnings that can become errors).
  - Small, self-contained change in one function and one file.
  - No functional side effects; does not alter behavior except removing
    undefined initialization state.
  - Not a feature or refactor; low regression risk; localized to RISC-V
    BPF trampoline.

Conclusion: This is a good and safe candidate for backporting to all
stable trees that include the RISC-V BPF trampoline code path (e.g.,
6.6.y and newer where applicable).

 arch/riscv/net/bpf_jit_comp64.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/arch/riscv/net/bpf_jit_comp64.c b/arch/riscv/net/bpf_jit_comp64.c
index 9883a55d61b5b..8475a8ab57151 100644
--- a/arch/riscv/net/bpf_jit_comp64.c
+++ b/arch/riscv/net/bpf_jit_comp64.c
@@ -1079,10 +1079,9 @@ static int __arch_prepare_bpf_trampoline(struct bpf_tramp_image *im,
 	stack_size += 16;
 
 	save_ret = flags & (BPF_TRAMP_F_CALL_ORIG | BPF_TRAMP_F_RET_FENTRY_RET);
-	if (save_ret) {
+	if (save_ret)
 		stack_size += 16; /* Save both A5 (BPF R0) and A0 */
-		retval_off = stack_size;
-	}
+	retval_off = stack_size;
 
 	stack_size += nr_arg_slots * 8;
 	args_off = stack_size;
-- 
2.51.0


      parent reply	other threads:[~2025-10-09 16:01 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20251009155752.773732-1-sashal@kernel.org>
2025-10-09 15:54 ` [PATCH AUTOSEL 6.17-5.4] bpf: Don't use %pK through printk Sasha Levin
2025-10-09 15:54 ` [PATCH AUTOSEL 6.17-6.1] bpftool: Fix -Wuninitialized-const-pointer warnings with clang >= 21 Sasha Levin
2025-10-09 15:54 ` [PATCH AUTOSEL 6.17-6.12] bpf: Use tnums for JEQ/JNE is_branch_taken logic Sasha Levin
2025-10-09 15:54 ` [PATCH AUTOSEL 6.17-6.16] selftests/bpf: Fix incorrect array size calculation Sasha Levin
2025-10-09 15:54 ` [PATCH AUTOSEL 6.17-6.12] selftests/bpf: Fix selftest verifier_arena_large failure Sasha Levin
2025-10-09 15:55 ` [PATCH AUTOSEL 6.17-6.1] bpf: Clear pfmemalloc flag when freeing all fragments Sasha Levin
2025-10-09 15:55 ` [PATCH AUTOSEL 6.17-5.4] selftests/bpf: Fix bpf_prog_detach2 usage in test_lirc_mode2 Sasha Levin
2025-10-09 15:55 ` [PATCH AUTOSEL 6.17-6.12] selftests/bpf: Fix flaky bpf_cookie selftest Sasha Levin
2025-10-09 15:55 ` [PATCH AUTOSEL 6.17] selftests: drv-net: Pull data before parsing headers Sasha Levin
2025-10-09 15:55 ` [PATCH AUTOSEL 6.17-6.12] libbpf: Fix USDT SIB argument handling causing unrecognized register error Sasha Levin
2025-10-09 15:55 ` [PATCH AUTOSEL 6.17-6.12] bpftool: Add CET-aware symbol matching for x86_64 architectures Sasha Levin
2025-10-09 15:55 ` [PATCH AUTOSEL 6.17-6.6] bpf: Do not limit bpf_cgroup_from_id to current's namespace Sasha Levin
2025-10-09 15:55 ` [PATCH AUTOSEL 6.17-6.1] selftests/bpf: Upon failures, exit with code 1 in test_xsk.sh Sasha Levin
2025-10-09 15:55 ` [PATCH AUTOSEL 6.17-6.16] selftests/bpf: Fix arena_spin_lock selftest failure Sasha Levin
2025-10-09 15:56 ` Sasha Levin [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20251009155752.773732-110-sashal@kernel.org \
    --to=sashal@kernel.org \
    --cc=andrii@kernel.org \
    --cc=aou@eecs.berkeley.edu \
    --cc=ast@kernel.org \
    --cc=bjorn@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=duanchenghao@kylinos.cn \
    --cc=linux-riscv@lists.infradead.org \
    --cc=palmer@dabbelt.com \
    --cc=patches@lists.linux.dev \
    --cc=pjw@kernel.org \
    --cc=pulehui@huawei.com \
    --cc=stable@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox