* [PATCH] LoongArch: BPF: Fix uninitialized symbol 'retval_off'
@ 2025-08-19 11:19 Huacai Chen
2025-08-22 18:43 ` Andrii Nakryiko
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Huacai Chen @ 2025-08-19 11:19 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann, John Fastabend
Cc: bpf, Tiezhu Yang, Hengqi Chen, Huacai Chen, George Guo,
Chenghao Duan, loongarch, Huacai Chen, kernel test robot,
Dan Carpenter
In __arch_prepare_bpf_trampoline(), retval_off is meaningful only when
save_ret is not 0, so the current logic is correct. But it may cause a
build warning:
arch/loongarch/net/bpf_jit.c:1547 __arch_prepare_bpf_trampoline() error: uninitialized symbol 'retval_off'.
So initialize retval_off unconditionally to fix it.
Fixes: f9b6b41f0cf3 ("LoongArch: BPF: Add basic bpf trampoline support")
Closes: https://lore.kernel.org/r/202508191020.PBBh07cK-lkp@intel.com/
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
Signed-off-by: Huacai Chen <chenhuacai@loongson.cn>
---
arch/loongarch/net/bpf_jit.c | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/arch/loongarch/net/bpf_jit.c b/arch/loongarch/net/bpf_jit.c
index abfdb6bb5c38..a73f6ea4ed4a 100644
--- a/arch/loongarch/net/bpf_jit.c
+++ b/arch/loongarch/net/bpf_jit.c
@@ -1504,11 +1504,10 @@ static int __arch_prepare_bpf_trampoline(struct jit_ctx *ctx, struct bpf_tramp_i
stack_size += 16;
save_ret = flags & (BPF_TRAMP_F_CALL_ORIG | BPF_TRAMP_F_RET_FENTRY_RET);
- if (save_ret) {
- /* Save BPF R0 and A0 */
- stack_size += 16;
- retval_off = stack_size;
- }
+ if (save_ret)
+ stack_size += 16; /* Save BPF R0 and A0 */
+
+ retval_off = stack_size;
/* Room of trampoline frame to store args */
nargs = m->nr_args;
--
2.47.3
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] LoongArch: BPF: Fix uninitialized symbol 'retval_off'
2025-08-19 11:19 [PATCH] LoongArch: BPF: Fix uninitialized symbol 'retval_off' Huacai Chen
@ 2025-08-22 18:43 ` Andrii Nakryiko
2025-08-23 4:10 ` Huacai Chen
2025-08-26 7:07 ` Tiezhu Yang
2025-08-26 9:41 ` Dan Carpenter
2 siblings, 1 reply; 6+ messages in thread
From: Andrii Nakryiko @ 2025-08-22 18:43 UTC (permalink / raw)
To: Huacai Chen
Cc: Alexei Starovoitov, Daniel Borkmann, John Fastabend, bpf,
Tiezhu Yang, Hengqi Chen, Huacai Chen, George Guo, Chenghao Duan,
loongarch, kernel test robot, Dan Carpenter
On Tue, Aug 19, 2025 at 4:21 AM Huacai Chen <chenhuacai@loongson.cn> wrote:
>
> In __arch_prepare_bpf_trampoline(), retval_off is meaningful only when
> save_ret is not 0, so the current logic is correct. But it may cause a
> build warning:
>
> arch/loongarch/net/bpf_jit.c:1547 __arch_prepare_bpf_trampoline() error: uninitialized symbol 'retval_off'.
>
> So initialize retval_off unconditionally to fix it.
>
> Fixes: f9b6b41f0cf3 ("LoongArch: BPF: Add basic bpf trampoline support")
> Closes: https://lore.kernel.org/r/202508191020.PBBh07cK-lkp@intel.com/
> Reported-by: kernel test robot <lkp@intel.com>
> Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
> Signed-off-by: Huacai Chen <chenhuacai@loongson.cn>
> ---
> arch/loongarch/net/bpf_jit.c | 9 ++++-----
> 1 file changed, 4 insertions(+), 5 deletions(-)
>
Is this something that should go through loongarch-specific tree?
> diff --git a/arch/loongarch/net/bpf_jit.c b/arch/loongarch/net/bpf_jit.c
> index abfdb6bb5c38..a73f6ea4ed4a 100644
> --- a/arch/loongarch/net/bpf_jit.c
> +++ b/arch/loongarch/net/bpf_jit.c
> @@ -1504,11 +1504,10 @@ static int __arch_prepare_bpf_trampoline(struct jit_ctx *ctx, struct bpf_tramp_i
> stack_size += 16;
>
> save_ret = flags & (BPF_TRAMP_F_CALL_ORIG | BPF_TRAMP_F_RET_FENTRY_RET);
> - if (save_ret) {
> - /* Save BPF R0 and A0 */
> - stack_size += 16;
> - retval_off = stack_size;
> - }
> + if (save_ret)
> + stack_size += 16; /* Save BPF R0 and A0 */
> +
> + retval_off = stack_size;
>
> /* Room of trampoline frame to store args */
> nargs = m->nr_args;
> --
> 2.47.3
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] LoongArch: BPF: Fix uninitialized symbol 'retval_off'
2025-08-22 18:43 ` Andrii Nakryiko
@ 2025-08-23 4:10 ` Huacai Chen
0 siblings, 0 replies; 6+ messages in thread
From: Huacai Chen @ 2025-08-23 4:10 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: Huacai Chen, Alexei Starovoitov, Daniel Borkmann, John Fastabend,
bpf, Tiezhu Yang, Hengqi Chen, George Guo, Chenghao Duan,
loongarch, kernel test robot, Dan Carpenter
Hi, Andrii,
On Sat, Aug 23, 2025 at 2:43 AM Andrii Nakryiko
<andrii.nakryiko@gmail.com> wrote:
>
> On Tue, Aug 19, 2025 at 4:21 AM Huacai Chen <chenhuacai@loongson.cn> wrote:
> >
> > In __arch_prepare_bpf_trampoline(), retval_off is meaningful only when
> > save_ret is not 0, so the current logic is correct. But it may cause a
> > build warning:
> >
> > arch/loongarch/net/bpf_jit.c:1547 __arch_prepare_bpf_trampoline() error: uninitialized symbol 'retval_off'.
> >
> > So initialize retval_off unconditionally to fix it.
> >
> > Fixes: f9b6b41f0cf3 ("LoongArch: BPF: Add basic bpf trampoline support")
> > Closes: https://lore.kernel.org/r/202508191020.PBBh07cK-lkp@intel.com/
> > Reported-by: kernel test robot <lkp@intel.com>
> > Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
> > Signed-off-by: Huacai Chen <chenhuacai@loongson.cn>
> > ---
> > arch/loongarch/net/bpf_jit.c | 9 ++++-----
> > 1 file changed, 4 insertions(+), 5 deletions(-)
> >
>
> Is this something that should go through loongarch-specific tree?
This one, and other LoongArch specific BPF patches will go through
loongarch tree, thanks.
Huacai
>
> > diff --git a/arch/loongarch/net/bpf_jit.c b/arch/loongarch/net/bpf_jit.c
> > index abfdb6bb5c38..a73f6ea4ed4a 100644
> > --- a/arch/loongarch/net/bpf_jit.c
> > +++ b/arch/loongarch/net/bpf_jit.c
> > @@ -1504,11 +1504,10 @@ static int __arch_prepare_bpf_trampoline(struct jit_ctx *ctx, struct bpf_tramp_i
> > stack_size += 16;
> >
> > save_ret = flags & (BPF_TRAMP_F_CALL_ORIG | BPF_TRAMP_F_RET_FENTRY_RET);
> > - if (save_ret) {
> > - /* Save BPF R0 and A0 */
> > - stack_size += 16;
> > - retval_off = stack_size;
> > - }
> > + if (save_ret)
> > + stack_size += 16; /* Save BPF R0 and A0 */
> > +
> > + retval_off = stack_size;
> >
> > /* Room of trampoline frame to store args */
> > nargs = m->nr_args;
> > --
> > 2.47.3
> >
> >
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] LoongArch: BPF: Fix uninitialized symbol 'retval_off'
2025-08-19 11:19 [PATCH] LoongArch: BPF: Fix uninitialized symbol 'retval_off' Huacai Chen
2025-08-22 18:43 ` Andrii Nakryiko
@ 2025-08-26 7:07 ` Tiezhu Yang
2025-08-26 8:40 ` Huacai Chen
2025-08-26 9:41 ` Dan Carpenter
2 siblings, 1 reply; 6+ messages in thread
From: Tiezhu Yang @ 2025-08-26 7:07 UTC (permalink / raw)
To: Huacai Chen, Alexei Starovoitov, Daniel Borkmann, John Fastabend
Cc: bpf, Hengqi Chen, Huacai Chen, George Guo, Chenghao Duan,
loongarch, kernel test robot, Dan Carpenter
On 2025/8/19 下午7:19, Huacai Chen wrote:
> In __arch_prepare_bpf_trampoline(), retval_off is meaningful only when
> save_ret is not 0, so the current logic is correct. But it may cause a
> build warning:
>
> arch/loongarch/net/bpf_jit.c:1547 __arch_prepare_bpf_trampoline() error: uninitialized symbol 'retval_off'.
>
> So initialize retval_off unconditionally to fix it.
>
> Fixes: f9b6b41f0cf3 ("LoongArch: BPF: Add basic bpf trampoline support")
> Closes: https://lore.kernel.org/r/202508191020.PBBh07cK-lkp@intel.com/
> Reported-by: kernel test robot <lkp@intel.com>
> Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
> Signed-off-by: Huacai Chen <chenhuacai@loongson.cn>
> ---
> arch/loongarch/net/bpf_jit.c | 9 ++++-----
> 1 file changed, 4 insertions(+), 5 deletions(-)
>
> diff --git a/arch/loongarch/net/bpf_jit.c b/arch/loongarch/net/bpf_jit.c
> index abfdb6bb5c38..a73f6ea4ed4a 100644
> --- a/arch/loongarch/net/bpf_jit.c
> +++ b/arch/loongarch/net/bpf_jit.c
> @@ -1504,11 +1504,10 @@ static int __arch_prepare_bpf_trampoline(struct jit_ctx *ctx, struct bpf_tramp_i
> stack_size += 16;
>
> save_ret = flags & (BPF_TRAMP_F_CALL_ORIG | BPF_TRAMP_F_RET_FENTRY_RET);
> - if (save_ret) {
> - /* Save BPF R0 and A0 */
> - stack_size += 16;
> - retval_off = stack_size;
> - }
> + if (save_ret)
> + stack_size += 16; /* Save BPF R0 and A0 */
> +
> + retval_off = stack_size;
Just init retval_off as 0 at the beginning of this function?
What is the difference? which is better?
Thanks,
Tiezhu
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] LoongArch: BPF: Fix uninitialized symbol 'retval_off'
2025-08-26 7:07 ` Tiezhu Yang
@ 2025-08-26 8:40 ` Huacai Chen
0 siblings, 0 replies; 6+ messages in thread
From: Huacai Chen @ 2025-08-26 8:40 UTC (permalink / raw)
To: Tiezhu Yang
Cc: Huacai Chen, Alexei Starovoitov, Daniel Borkmann, John Fastabend,
bpf, Hengqi Chen, George Guo, Chenghao Duan, loongarch,
kernel test robot, Dan Carpenter
On Tue, Aug 26, 2025 at 3:08 PM Tiezhu Yang <yangtiezhu@loongson.cn> wrote:
>
> On 2025/8/19 下午7:19, Huacai Chen wrote:
> > In __arch_prepare_bpf_trampoline(), retval_off is meaningful only when
> > save_ret is not 0, so the current logic is correct. But it may cause a
> > build warning:
> >
> > arch/loongarch/net/bpf_jit.c:1547 __arch_prepare_bpf_trampoline() error: uninitialized symbol 'retval_off'.
> >
> > So initialize retval_off unconditionally to fix it.
> >
> > Fixes: f9b6b41f0cf3 ("LoongArch: BPF: Add basic bpf trampoline support")
> > Closes: https://lore.kernel.org/r/202508191020.PBBh07cK-lkp@intel.com/
> > Reported-by: kernel test robot <lkp@intel.com>
> > Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
> > Signed-off-by: Huacai Chen <chenhuacai@loongson.cn>
> > ---
> > arch/loongarch/net/bpf_jit.c | 9 ++++-----
> > 1 file changed, 4 insertions(+), 5 deletions(-)
> >
> > diff --git a/arch/loongarch/net/bpf_jit.c b/arch/loongarch/net/bpf_jit.c
> > index abfdb6bb5c38..a73f6ea4ed4a 100644
> > --- a/arch/loongarch/net/bpf_jit.c
> > +++ b/arch/loongarch/net/bpf_jit.c
> > @@ -1504,11 +1504,10 @@ static int __arch_prepare_bpf_trampoline(struct jit_ctx *ctx, struct bpf_tramp_i
> > stack_size += 16;
> >
> > save_ret = flags & (BPF_TRAMP_F_CALL_ORIG | BPF_TRAMP_F_RET_FENTRY_RET);
> > - if (save_ret) {
> > - /* Save BPF R0 and A0 */
> > - stack_size += 16;
> > - retval_off = stack_size;
> > - }
> > + if (save_ret)
> > + stack_size += 16; /* Save BPF R0 and A0 */
> > +
> > + retval_off = stack_size;
>
> Just init retval_off as 0 at the beginning of this function?
> What is the difference? which is better?
This patch is more like ARM64 does.
In addition, if save_ret is true, "init retval_off as 0" causes
retval_off to be evaluated twice, while this patch is only evaluated
once. But surely this is not a big deal.
Huacai
>
> Thanks,
> Tiezhu
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] LoongArch: BPF: Fix uninitialized symbol 'retval_off'
2025-08-19 11:19 [PATCH] LoongArch: BPF: Fix uninitialized symbol 'retval_off' Huacai Chen
2025-08-22 18:43 ` Andrii Nakryiko
2025-08-26 7:07 ` Tiezhu Yang
@ 2025-08-26 9:41 ` Dan Carpenter
2 siblings, 0 replies; 6+ messages in thread
From: Dan Carpenter @ 2025-08-26 9:41 UTC (permalink / raw)
To: Huacai Chen
Cc: Alexei Starovoitov, Daniel Borkmann, John Fastabend, bpf,
Tiezhu Yang, Hengqi Chen, Huacai Chen, George Guo, Chenghao Duan,
loongarch, kernel test robot
On Tue, Aug 19, 2025 at 07:19:53PM +0800, Huacai Chen wrote:
> In __arch_prepare_bpf_trampoline(), retval_off is meaningful only when
> save_ret is not 0, so the current logic is correct. But it may cause a
> build warning:
We pass uninitialized data to invoke_bpf_prog(), it's just that it isn't
used.
In the kernel, we would say that's not a bug if invoke_bpf_prog() is
inlined. (The C standard doesn't differentiate between inlined and not
inlined, passing uninitialized variables is always considered undefined
behavior). UBSan will complain about the uninitialized variable at
runtime as well depending on if it's inlined or not.
It's not marked as an __always_inline function...
regards,
dan carpenter
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2025-08-26 9:41 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-08-19 11:19 [PATCH] LoongArch: BPF: Fix uninitialized symbol 'retval_off' Huacai Chen
2025-08-22 18:43 ` Andrii Nakryiko
2025-08-23 4:10 ` Huacai Chen
2025-08-26 7:07 ` Tiezhu Yang
2025-08-26 8:40 ` Huacai Chen
2025-08-26 9:41 ` Dan Carpenter
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).