bpf.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] LoongArch: BPF: Fix incorrect return pointer value in the eBPF program
@ 2025-08-14  1:34 Haoran Jiang
  2025-08-14  3:23 ` Huacai Chen
  2025-08-14 12:59 ` [PATCH] " Jinyang He
  0 siblings, 2 replies; 6+ messages in thread
From: Haoran Jiang @ 2025-08-14  1:34 UTC (permalink / raw)
  To: loongarch
  Cc: bpf, kernel, chenhuacai, hengqi.chen, yangtiezhu, jolsa, haoluo,
	sdf, kpsingh, john.fastabend, yonghong.song, song, eddyz87,
	martin.lau, andrii, daniel, ast

In some eBPF programs, the return value is a pointer.
When the kernel call an eBPF program (such as struct_ops),
it expects a 64-bit address to be returned, but instead a 32-bit value.

Before applying this patch:
./test_progs -a ns_bpf_qdisc
CPU 7 Unable to handle kernel paging request at virtual
address 0000000010440158.

As shown in the following test case,
bpf_fifo_dequeue return value is a pointer.
progs/bpf_qdisc_fifo.c

SEC("struct_ops/bpf_fifo_dequeue")
struct sk_buff *BPF_PROG(bpf_fifo_dequeue, struct Qdisc *sch)
{
	struct sk_buff *skb = NULL;
	........
	skb = bpf_kptr_xchg(&skbn->skb, skb);
	........
	return skb;
}

kernel call bpf_fifo_dequeue:
net/sched/sch_generic.c

static struct sk_buff *dequeue_skb(struct Qdisc *q, bool *validate,
				   int *packets)
{
	struct sk_buff *skb = NULL;
	........
	skb = q->dequeue(q);
	.........
}
When accessing the skb, an address exception error will occur.
because the value returned by q->dequeue at this point is a 32-bit
address rather than a 64-bit address.

After applying the patch:
./test_progs -a ns_bpf_qdisc
Warning: sch_htb: quantum of class 10001 is small. Consider r2q change.
213/1   ns_bpf_qdisc/fifo:OK
213/2   ns_bpf_qdisc/fq:OK
213/3   ns_bpf_qdisc/attach to mq:OK
213/4   ns_bpf_qdisc/attach to non root:OK
213/5   ns_bpf_qdisc/incompl_ops:OK
213     ns_bpf_qdisc:OK
Summary: 1/5 PASSED, 0 SKIPPED, 0 FAILED

Fixes: 73c359d1d356 ("LoongArch: BPF: Sign-extend return values")
Signed-off-by: Haoran Jiang <jianghaoran@kylinos.cn>
---
 arch/loongarch/net/bpf_jit.c | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/arch/loongarch/net/bpf_jit.c b/arch/loongarch/net/bpf_jit.c
index abfdb6bb5c38..7df067a42f36 100644
--- a/arch/loongarch/net/bpf_jit.c
+++ b/arch/loongarch/net/bpf_jit.c
@@ -229,8 +229,24 @@ static void __build_epilogue(struct jit_ctx *ctx, bool is_tail_call)
 	emit_insn(ctx, addid, LOONGARCH_GPR_SP, LOONGARCH_GPR_SP, stack_adjust);
 
 	if (!is_tail_call) {
-		/* Set return value */
+		/*
+		 *  Set return value
+		 *  Check if the 64th bit in regmap[BPF_REG_0] is 1. If it is,
+		 *  the value in regmap[BPF_REG_0] is a kernel-space address.
+		 *
+		 *  t1 = regmap[BPF_REG_0] >> 63
+		 *  t2 = 1
+		 *  if(t2 == t1)
+		 *	move a0 <- regmap[BPF_REG_0]
+		 *  else
+		 *	addiw a0 <- regmap[BPF_REG_0] + 0
+		 */
+		emit_insn(ctx, srlid, LOONGARCH_GPR_T1, regmap[BPF_REG_0], 63);
+		emit_insn(ctx, addid, LOONGARCH_GPR_T2, LOONGARCH_GPR_ZERO, 0x1);
+		emit_cond_jmp(ctx, BPF_JEQ, LOONGARCH_GPR_T1, LOONGARCH_GPR_T2, 3);
 		emit_insn(ctx, addiw, LOONGARCH_GPR_A0, regmap[BPF_REG_0], 0);
+		emit_uncond_jmp(ctx, 2);
+		move_reg(ctx, LOONGARCH_GPR_A0, regmap[BPF_REG_0]);
 		/* Return to the caller */
 		emit_insn(ctx, jirl, LOONGARCH_GPR_ZERO, LOONGARCH_GPR_RA, 0);
 	} else {
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] LoongArch: BPF: Fix incorrect return pointer value in the eBPF program
  2025-08-14  1:34 [PATCH] LoongArch: BPF: Fix incorrect return pointer value in the eBPF program Haoran Jiang
@ 2025-08-14  3:23 ` Huacai Chen
  2025-08-14  4:13   ` Vincent Li
  2025-08-14  5:40   ` Re:[PATCH] " jianghaoran
  2025-08-14 12:59 ` [PATCH] " Jinyang He
  1 sibling, 2 replies; 6+ messages in thread
From: Huacai Chen @ 2025-08-14  3:23 UTC (permalink / raw)
  To: Haoran Jiang
  Cc: loongarch, bpf, kernel, hengqi.chen, yangtiezhu, jolsa, haoluo,
	sdf, kpsingh, john.fastabend, yonghong.song, song, eddyz87,
	martin.lau, andrii, daniel, ast

Hi, Haoran,

On Thu, Aug 14, 2025 at 9:34 AM Haoran Jiang <jianghaoran@kylinos.cn> wrote:
>
> In some eBPF programs, the return value is a pointer.
> When the kernel call an eBPF program (such as struct_ops),
> it expects a 64-bit address to be returned, but instead a 32-bit value.
>
> Before applying this patch:
> ./test_progs -a ns_bpf_qdisc
> CPU 7 Unable to handle kernel paging request at virtual
> address 0000000010440158.
>
> As shown in the following test case,
> bpf_fifo_dequeue return value is a pointer.
> progs/bpf_qdisc_fifo.c
>
> SEC("struct_ops/bpf_fifo_dequeue")
> struct sk_buff *BPF_PROG(bpf_fifo_dequeue, struct Qdisc *sch)
> {
>         struct sk_buff *skb = NULL;
>         ........
>         skb = bpf_kptr_xchg(&skbn->skb, skb);
>         ........
>         return skb;
> }
>
> kernel call bpf_fifo_dequeue:
> net/sched/sch_generic.c
>
> static struct sk_buff *dequeue_skb(struct Qdisc *q, bool *validate,
>                                    int *packets)
> {
>         struct sk_buff *skb = NULL;
>         ........
>         skb = q->dequeue(q);
>         .........
> }
> When accessing the skb, an address exception error will occur.
> because the value returned by q->dequeue at this point is a 32-bit
> address rather than a 64-bit address.
>
> After applying the patch:
> ./test_progs -a ns_bpf_qdisc
> Warning: sch_htb: quantum of class 10001 is small. Consider r2q change.
> 213/1   ns_bpf_qdisc/fifo:OK
> 213/2   ns_bpf_qdisc/fq:OK
> 213/3   ns_bpf_qdisc/attach to mq:OK
> 213/4   ns_bpf_qdisc/attach to non root:OK
> 213/5   ns_bpf_qdisc/incompl_ops:OK
> 213     ns_bpf_qdisc:OK
> Summary: 1/5 PASSED, 0 SKIPPED, 0 FAILED
>
> Fixes: 73c359d1d356 ("LoongArch: BPF: Sign-extend return values")
> Signed-off-by: Haoran Jiang <jianghaoran@kylinos.cn>
Can this patch solve this bug?
https://lore.kernel.org/loongarch/CAK3+h2x1gjuqEsUSj+B-9sb73kRo3bStH6ROw=1LVSqQGMNcUw@mail.gmail.com/T/#t

Huacai

> ---
>  arch/loongarch/net/bpf_jit.c | 18 +++++++++++++++++-
>  1 file changed, 17 insertions(+), 1 deletion(-)
>
> diff --git a/arch/loongarch/net/bpf_jit.c b/arch/loongarch/net/bpf_jit.c
> index abfdb6bb5c38..7df067a42f36 100644
> --- a/arch/loongarch/net/bpf_jit.c
> +++ b/arch/loongarch/net/bpf_jit.c
> @@ -229,8 +229,24 @@ static void __build_epilogue(struct jit_ctx *ctx, bool is_tail_call)
>         emit_insn(ctx, addid, LOONGARCH_GPR_SP, LOONGARCH_GPR_SP, stack_adjust);
>
>         if (!is_tail_call) {
> -               /* Set return value */
> +               /*
> +                *  Set return value
> +                *  Check if the 64th bit in regmap[BPF_REG_0] is 1. If it is,
> +                *  the value in regmap[BPF_REG_0] is a kernel-space address.
> +                *
> +                *  t1 = regmap[BPF_REG_0] >> 63
> +                *  t2 = 1
> +                *  if(t2 == t1)
> +                *      move a0 <- regmap[BPF_REG_0]
> +                *  else
> +                *      addiw a0 <- regmap[BPF_REG_0] + 0
> +                */
> +               emit_insn(ctx, srlid, LOONGARCH_GPR_T1, regmap[BPF_REG_0], 63);
> +               emit_insn(ctx, addid, LOONGARCH_GPR_T2, LOONGARCH_GPR_ZERO, 0x1);
> +               emit_cond_jmp(ctx, BPF_JEQ, LOONGARCH_GPR_T1, LOONGARCH_GPR_T2, 3);
>                 emit_insn(ctx, addiw, LOONGARCH_GPR_A0, regmap[BPF_REG_0], 0);
> +               emit_uncond_jmp(ctx, 2);
> +               move_reg(ctx, LOONGARCH_GPR_A0, regmap[BPF_REG_0]);
>                 /* Return to the caller */
>                 emit_insn(ctx, jirl, LOONGARCH_GPR_ZERO, LOONGARCH_GPR_RA, 0);
>         } else {
> --
> 2.43.0
>
>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] LoongArch: BPF: Fix incorrect return pointer value in the eBPF program
  2025-08-14  3:23 ` Huacai Chen
@ 2025-08-14  4:13   ` Vincent Li
  2025-08-14  5:40   ` Re:[PATCH] " jianghaoran
  1 sibling, 0 replies; 6+ messages in thread
From: Vincent Li @ 2025-08-14  4:13 UTC (permalink / raw)
  To: Huacai Chen
  Cc: Haoran Jiang, loongarch, bpf, kernel, hengqi.chen, yangtiezhu,
	jolsa, haoluo, sdf, kpsingh, john.fastabend, yonghong.song, song,
	eddyz87, martin.lau, andrii, daniel, ast

On Wed, Aug 13, 2025 at 8:24 PM Huacai Chen <chenhuacai@kernel.org> wrote:
>
> Hi, Haoran,
>
> On Thu, Aug 14, 2025 at 9:34 AM Haoran Jiang <jianghaoran@kylinos.cn> wrote:
> >
> > In some eBPF programs, the return value is a pointer.
> > When the kernel call an eBPF program (such as struct_ops),
> > it expects a 64-bit address to be returned, but instead a 32-bit value.
> >
> > Before applying this patch:
> > ./test_progs -a ns_bpf_qdisc
> > CPU 7 Unable to handle kernel paging request at virtual
> > address 0000000010440158.
> >
> > As shown in the following test case,
> > bpf_fifo_dequeue return value is a pointer.
> > progs/bpf_qdisc_fifo.c
> >
> > SEC("struct_ops/bpf_fifo_dequeue")
> > struct sk_buff *BPF_PROG(bpf_fifo_dequeue, struct Qdisc *sch)
> > {
> >         struct sk_buff *skb = NULL;
> >         ........
> >         skb = bpf_kptr_xchg(&skbn->skb, skb);
> >         ........
> >         return skb;
> > }
> >
> > kernel call bpf_fifo_dequeue:
> > net/sched/sch_generic.c
> >
> > static struct sk_buff *dequeue_skb(struct Qdisc *q, bool *validate,
> >                                    int *packets)
> > {
> >         struct sk_buff *skb = NULL;
> >         ........
> >         skb = q->dequeue(q);
> >         .........
> > }
> > When accessing the skb, an address exception error will occur.
> > because the value returned by q->dequeue at this point is a 32-bit
> > address rather than a 64-bit address.
> >
> > After applying the patch:
> > ./test_progs -a ns_bpf_qdisc
> > Warning: sch_htb: quantum of class 10001 is small. Consider r2q change.
> > 213/1   ns_bpf_qdisc/fifo:OK
> > 213/2   ns_bpf_qdisc/fq:OK
> > 213/3   ns_bpf_qdisc/attach to mq:OK
> > 213/4   ns_bpf_qdisc/attach to non root:OK
> > 213/5   ns_bpf_qdisc/incompl_ops:OK
> > 213     ns_bpf_qdisc:OK
> > Summary: 1/5 PASSED, 0 SKIPPED, 0 FAILED
> >
> > Fixes: 73c359d1d356 ("LoongArch: BPF: Sign-extend return values")
> > Signed-off-by: Haoran Jiang <jianghaoran@kylinos.cn>
> Can this patch solve this bug?
> https://lore.kernel.org/loongarch/CAK3+h2x1gjuqEsUSj+B-9sb73kRo3bStH6ROw=1LVSqQGMNcUw@mail.gmail.com/T/#t
>

I tested this patch, it does not solve bpf selftests module_attach lockup issue.

> Huacai
>
> > ---
> >  arch/loongarch/net/bpf_jit.c | 18 +++++++++++++++++-
> >  1 file changed, 17 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/loongarch/net/bpf_jit.c b/arch/loongarch/net/bpf_jit.c
> > index abfdb6bb5c38..7df067a42f36 100644
> > --- a/arch/loongarch/net/bpf_jit.c
> > +++ b/arch/loongarch/net/bpf_jit.c
> > @@ -229,8 +229,24 @@ static void __build_epilogue(struct jit_ctx *ctx, bool is_tail_call)
> >         emit_insn(ctx, addid, LOONGARCH_GPR_SP, LOONGARCH_GPR_SP, stack_adjust);
> >
> >         if (!is_tail_call) {
> > -               /* Set return value */
> > +               /*
> > +                *  Set return value
> > +                *  Check if the 64th bit in regmap[BPF_REG_0] is 1. If it is,
> > +                *  the value in regmap[BPF_REG_0] is a kernel-space address.
> > +                *
> > +                *  t1 = regmap[BPF_REG_0] >> 63
> > +                *  t2 = 1
> > +                *  if(t2 == t1)
> > +                *      move a0 <- regmap[BPF_REG_0]
> > +                *  else
> > +                *      addiw a0 <- regmap[BPF_REG_0] + 0
> > +                */
> > +               emit_insn(ctx, srlid, LOONGARCH_GPR_T1, regmap[BPF_REG_0], 63);
> > +               emit_insn(ctx, addid, LOONGARCH_GPR_T2, LOONGARCH_GPR_ZERO, 0x1);
> > +               emit_cond_jmp(ctx, BPF_JEQ, LOONGARCH_GPR_T1, LOONGARCH_GPR_T2, 3);
> >                 emit_insn(ctx, addiw, LOONGARCH_GPR_A0, regmap[BPF_REG_0], 0);
> > +               emit_uncond_jmp(ctx, 2);
> > +               move_reg(ctx, LOONGARCH_GPR_A0, regmap[BPF_REG_0]);
> >                 /* Return to the caller */
> >                 emit_insn(ctx, jirl, LOONGARCH_GPR_ZERO, LOONGARCH_GPR_RA, 0);
> >         } else {
> > --
> > 2.43.0
> >
> >
>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re:[PATCH] LoongArch: BPF: Fix incorrect return pointer value in the eBPF program
  2025-08-14  3:23 ` Huacai Chen
  2025-08-14  4:13   ` Vincent Li
@ 2025-08-14  5:40   ` jianghaoran
  1 sibling, 0 replies; 6+ messages in thread
From: jianghaoran @ 2025-08-14  5:40 UTC (permalink / raw)
  To: Huacai Chen
  Cc: loongarch, bpf, kernel, hengqi.chen, yangtiezhu, jolsa, haoluo,
	sdf, kpsingh, john.fastabend, yonghong.song, song, eddyz87,
	martin.lau, andrii, daniel, ast


在 2025-08-14星期四的 11:23 +0800,Huacai Chen写道:
> Hi, Haoran,
> 
> On Thu, Aug 14, 2025 at 9:34 AM Haoran Jiang <
> jianghaoran@kylinos.cn
> > wrote:
> > In some eBPF programs, the return value is a pointer.
> > When the kernel call an eBPF program (such as struct_ops),
> > it expects a 64-bit address to be returned, but instead a 32-
> > bit value.
> > 
> > Before applying this patch:
> > ./test_progs -a ns_bpf_qdisc
> > CPU 7 Unable to handle kernel paging request at virtual
> > address 0000000010440158.
> > 
> > As shown in the following test case,
> > bpf_fifo_dequeue return value is a pointer.
> > progs/bpf_qdisc_fifo.c
> > 
> > SEC("struct_ops/bpf_fifo_dequeue")
> > struct sk_buff *BPF_PROG(bpf_fifo_dequeue, struct Qdisc *sch)
> > {
> >         struct sk_buff *skb = NULL;
> >         ........
> >         skb = bpf_kptr_xchg(&skbn->skb, skb);
> >         ........
> >         return skb;
> > }
> > 
> > kernel call bpf_fifo_dequeue:
> > net/sched/sch_generic.c
> > 
> > static struct sk_buff *dequeue_skb(struct Qdisc *q, bool
> > *validate,
> >                                    int *packets)
> > {
> >         struct sk_buff *skb = NULL;
> >         ........
> >         skb = q->dequeue(q);
> >         .........
> > }
> > When accessing the skb, an address exception error will occur.
> > because the value returned by q->dequeue at this point is a 32-
> > bit
> > address rather than a 64-bit address.
> > 
> > After applying the patch:
> > ./test_progs -a ns_bpf_qdisc
> > Warning: sch_htb: quantum of class 10001 is small. Consider r2q
> > change.
> > 213/1   ns_bpf_qdisc/fifo:OK
> > 213/2   ns_bpf_qdisc/fq:OK
> > 213/3   ns_bpf_qdisc/attach to mq:OK
> > 213/4   ns_bpf_qdisc/attach to non root:OK
> > 213/5   ns_bpf_qdisc/incompl_ops:OK
> > 213     ns_bpf_qdisc:OK
> > Summary: 1/5 PASSED, 0 SKIPPED, 0 FAILED
> > 
> > Fixes: 73c359d1d356 ("LoongArch: BPF: Sign-extend return
> > values")
> > Signed-off-by: Haoran Jiang <
> > jianghaoran@kylinos.cn
> > >
> 
> Can this patch solve this bug?
> https://lore.kernel.org/loongarch/CAK3+h2x1gjuqEsUSj+B-9sb73kRo3bStH6ROw=1LVSqQGMNcUw@mail.gmail.com/T/#t
> 
> 
> Huacai

This patch can't  solve this problem. Currently, Chenghao is
researching this issue.

Haoran
> > ---
> >  arch/loongarch/net/bpf_jit.c | 18 +++++++++++++++++-
> >  1 file changed, 17 insertions(+), 1 deletion(-)
> > 
> > diff --git a/arch/loongarch/net/bpf_jit.c
> > b/arch/loongarch/net/bpf_jit.c
> > index abfdb6bb5c38..7df067a42f36 100644
> > --- a/arch/loongarch/net/bpf_jit.c
> > +++ b/arch/loongarch/net/bpf_jit.c
> > @@ -229,8 +229,24 @@ static void __build_epilogue(struct
> > jit_ctx *ctx, bool is_tail_call)
> >         emit_insn(ctx, addid, LOONGARCH_GPR_SP,
> > LOONGARCH_GPR_SP, stack_adjust);
> > 
> >         if (!is_tail_call) {
> > -               /* Set return value */
> > +               /*
> > +                *  Set return value
> > +                *  Check if the 64th bit in regmap[BPF_REG_0]
> > is 1. If it is,
> > +                *  the value in regmap[BPF_REG_0] is a kernel-
> > space address.
> > +                *
> > +                *  t1 = regmap[BPF_REG_0] >> 63
> > +                *  t2 = 1
> > +                *  if(t2 == t1)
> > +                *      move a0 <- regmap[BPF_REG_0]
> > +                *  else
> > +                *      addiw a0 <- regmap[BPF_REG_0] + 0
> > +                */
> > +               emit_insn(ctx, srlid, LOONGARCH_GPR_T1,
> > regmap[BPF_REG_0], 63);
> > +               emit_insn(ctx, addid, LOONGARCH_GPR_T2,
> > LOONGARCH_GPR_ZERO, 0x1);
> > +               emit_cond_jmp(ctx, BPF_JEQ, LOONGARCH_GPR_T1,
> > LOONGARCH_GPR_T2, 3);
> >                 emit_insn(ctx, addiw, LOONGARCH_GPR_A0,
> > regmap[BPF_REG_0], 0);
> > +               emit_uncond_jmp(ctx, 2);
> > +               move_reg(ctx, LOONGARCH_GPR_A0,
> > regmap[BPF_REG_0]);
> >                 /* Return to the caller */
> >                 emit_insn(ctx, jirl, LOONGARCH_GPR_ZERO,
> > LOONGARCH_GPR_RA, 0);
> >         } else {
> > --
> > 2.43.0
> > 
> > 


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] LoongArch: BPF: Fix incorrect return pointer value in the eBPF program
  2025-08-14  1:34 [PATCH] LoongArch: BPF: Fix incorrect return pointer value in the eBPF program Haoran Jiang
  2025-08-14  3:23 ` Huacai Chen
@ 2025-08-14 12:59 ` Jinyang He
  2025-08-15  5:11   ` 回复:[PATCH] " jianghaoran
  1 sibling, 1 reply; 6+ messages in thread
From: Jinyang He @ 2025-08-14 12:59 UTC (permalink / raw)
  To: Haoran Jiang, loongarch
  Cc: bpf, kernel, chenhuacai, hengqi.chen, yangtiezhu, jolsa, haoluo,
	sdf, kpsingh, john.fastabend, yonghong.song, song, eddyz87,
	martin.lau, andrii, daniel, ast

On 2025-08-14 09:34, Haoran Jiang wrote:

> In some eBPF programs, the return value is a pointer.
> When the kernel call an eBPF program (such as struct_ops),
> it expects a 64-bit address to be returned, but instead a 32-bit value.
>
> Before applying this patch:
> ./test_progs -a ns_bpf_qdisc
> CPU 7 Unable to handle kernel paging request at virtual
> address 0000000010440158.
>
> As shown in the following test case,
> bpf_fifo_dequeue return value is a pointer.
> progs/bpf_qdisc_fifo.c
>
> SEC("struct_ops/bpf_fifo_dequeue")
> struct sk_buff *BPF_PROG(bpf_fifo_dequeue, struct Qdisc *sch)
> {
> 	struct sk_buff *skb = NULL;
> 	........
> 	skb = bpf_kptr_xchg(&skbn->skb, skb);
> 	........
> 	return skb;
> }
>
> kernel call bpf_fifo_dequeue:
> net/sched/sch_generic.c
>
> static struct sk_buff *dequeue_skb(struct Qdisc *q, bool *validate,
> 				   int *packets)
> {
> 	struct sk_buff *skb = NULL;
> 	........
> 	skb = q->dequeue(q);
> 	.........
> }
> When accessing the skb, an address exception error will occur.
> because the value returned by q->dequeue at this point is a 32-bit
> address rather than a 64-bit address.
>
> After applying the patch:
> ./test_progs -a ns_bpf_qdisc
> Warning: sch_htb: quantum of class 10001 is small. Consider r2q change.
> 213/1   ns_bpf_qdisc/fifo:OK
> 213/2   ns_bpf_qdisc/fq:OK
> 213/3   ns_bpf_qdisc/attach to mq:OK
> 213/4   ns_bpf_qdisc/attach to non root:OK
> 213/5   ns_bpf_qdisc/incompl_ops:OK
> 213     ns_bpf_qdisc:OK
> Summary: 1/5 PASSED, 0 SKIPPED, 0 FAILED
>
> Fixes: 73c359d1d356 ("LoongArch: BPF: Sign-extend return values")
> Signed-off-by: Haoran Jiang <jianghaoran@kylinos.cn>
> ---
>   arch/loongarch/net/bpf_jit.c | 18 +++++++++++++++++-
>   1 file changed, 17 insertions(+), 1 deletion(-)
>
> diff --git a/arch/loongarch/net/bpf_jit.c b/arch/loongarch/net/bpf_jit.c
> index abfdb6bb5c38..7df067a42f36 100644
> --- a/arch/loongarch/net/bpf_jit.c
> +++ b/arch/loongarch/net/bpf_jit.c
> @@ -229,8 +229,24 @@ static void __build_epilogue(struct jit_ctx *ctx, bool is_tail_call)
>   	emit_insn(ctx, addid, LOONGARCH_GPR_SP, LOONGARCH_GPR_SP, stack_adjust);
>   
>   	if (!is_tail_call) {
> -		/* Set return value */
> +		/*
> +		 *  Set return value
> +		 *  Check if the 64th bit in regmap[BPF_REG_0] is 1. If it is,
> +		 *  the value in regmap[BPF_REG_0] is a kernel-space address.
> +		 *
> +		 *  t1 = regmap[BPF_REG_0] >> 63
> +		 *  t2 = 1
> +		 *  if(t2 == t1)
> +		 *	move a0 <- regmap[BPF_REG_0]
> +		 *  else
> +		 *	addiw a0 <- regmap[BPF_REG_0] + 0
> +		 */
> +		emit_insn(ctx, srlid, LOONGARCH_GPR_T1, regmap[BPF_REG_0], 63);
> +		emit_insn(ctx, addid, LOONGARCH_GPR_T2, LOONGARCH_GPR_ZERO, 0x1);
> +		emit_cond_jmp(ctx, BPF_JEQ, LOONGARCH_GPR_T1, LOONGARCH_GPR_T2, 3);

Hi, Haoran,

Just for codegen, we have many ways to avoid branch. Follows is a 
possible way.

long long val = regmap[BPF_REG_0];
int shift = 0 < val ? 32 : 0;
return (val << shift) >> shift;

slt    t0, zero, BPF_REG_0
slli.d t0, t0, 5
sll.d  BPF_REG_0, BPF_REG_0, t0
sra.d  BPF_REG_0, BPF_REG_0, t0


>   		emit_insn(ctx, addiw, LOONGARCH_GPR_A0, regmap[BPF_REG_0], 0);
> +		emit_uncond_jmp(ctx, 2);
> +		move_reg(ctx, LOONGARCH_GPR_A0, regmap[BPF_REG_0]);
>   		/* Return to the caller */
>   		emit_insn(ctx, jirl, LOONGARCH_GPR_ZERO, LOONGARCH_GPR_RA, 0);
>   	} else {


^ permalink raw reply	[flat|nested] 6+ messages in thread

* 回复:[PATCH] LoongArch: BPF: Fix incorrect return pointer value in the eBPF program
  2025-08-14 12:59 ` [PATCH] " Jinyang He
@ 2025-08-15  5:11   ` jianghaoran
  0 siblings, 0 replies; 6+ messages in thread
From: jianghaoran @ 2025-08-15  5:11 UTC (permalink / raw)
  To: Jinyang He, loongarch
  Cc: bpf, kernel, chenhuacai, hengqi.chen, yangtiezhu, jolsa, haoluo,
	sdf, kpsingh, john.fastabend, yonghong.song, song, eddyz87,
	martin.lau, andrii, daniel, ast


在 2025-08-14星期四的 20:59 +0800,Jinyang He写道:
> On 2025-08-14 09:34, Haoran Jiang wrote:
> 
> > In some eBPF programs, the return value is a pointer.
> > When the kernel call an eBPF program (such as struct_ops),
> > it expects a 64-bit address to be returned, but instead a 32-
> > bit value.
> > 
> > Before applying this patch:
> > ./test_progs -a ns_bpf_qdisc
> > CPU 7 Unable to handle kernel paging request at virtual
> > address 0000000010440158.
> > 
> > As shown in the following test case,
> > bpf_fifo_dequeue return value is a pointer.
> > progs/bpf_qdisc_fifo.c
> > 
> > SEC("struct_ops/bpf_fifo_dequeue")
> > struct sk_buff *BPF_PROG(bpf_fifo_dequeue, struct Qdisc *sch)
> > {
> > 	struct sk_buff *skb = NULL;
> > 	........
> > 	skb = bpf_kptr_xchg(&skbn->skb, skb);
> > 	........
> > 	return skb;
> > }
> > 
> > kernel call bpf_fifo_dequeue:
> > net/sched/sch_generic.c
> > 
> > static struct sk_buff *dequeue_skb(struct Qdisc *q, bool
> > *validate,
> > 				   int *packets)
> > {
> > 	struct sk_buff *skb = NULL;
> > 	........
> > 	skb = q->dequeue(q);
> > 	.........
> > }
> > When accessing the skb, an address exception error will occur.
> > because the value returned by q->dequeue at this point is a 32-
> > bit
> > address rather than a 64-bit address.
> > 
> > After applying the patch:
> > ./test_progs -a ns_bpf_qdisc
> > Warning: sch_htb: quantum of class 10001 is small. Consider r2q
> > change.
> > 213/1   ns_bpf_qdisc/fifo:OK
> > 213/2   ns_bpf_qdisc/fq:OK
> > 213/3   ns_bpf_qdisc/attach to mq:OK
> > 213/4   ns_bpf_qdisc/attach to non root:OK
> > 213/5   ns_bpf_qdisc/incompl_ops:OK
> > 213     ns_bpf_qdisc:OK
> > Summary: 1/5 PASSED, 0 SKIPPED, 0 FAILED
> > 
> > Fixes: 73c359d1d356 ("LoongArch: BPF: Sign-extend return
> > values")
> > Signed-off-by: Haoran Jiang <
> > jianghaoran@kylinos.cn
> > >
> > ---
> >   arch/loongarch/net/bpf_jit.c | 18 +++++++++++++++++-
> >   1 file changed, 17 insertions(+), 1 deletion(-)
> > 
> > diff --git a/arch/loongarch/net/bpf_jit.c
> > b/arch/loongarch/net/bpf_jit.c
> > index abfdb6bb5c38..7df067a42f36 100644
> > --- a/arch/loongarch/net/bpf_jit.c
> > +++ b/arch/loongarch/net/bpf_jit.c
> > @@ -229,8 +229,24 @@ static void __build_epilogue(struct
> > jit_ctx *ctx, bool is_tail_call)
> >   	emit_insn(ctx, addid, LOONGARCH_GPR_SP, LOONGARCH_GPR_SP,
> > stack_adjust);
> >   
> >   	if (!is_tail_call) {
> > -		/* Set return value */
> > +		/*
> > +		 *  Set return value
> > +		 *  Check if the 64th bit in regmap[BPF_REG_0] is
> > 1. If it is,
> > +		 *  the value in regmap[BPF_REG_0] is a kernel-
> > space address.
> > +		 *
> > +		 *  t1 = regmap[BPF_REG_0] >> 63
> > +		 *  t2 = 1
> > +		 *  if(t2 == t1)
> > +		 *	move a0 <- regmap[BPF_REG_0]
> > +		 *  else
> > +		 *	addiw a0 <- regmap[BPF_REG_0] + 0
> > +		 */
> > +		emit_insn(ctx, srlid, LOONGARCH_GPR_T1,
> > regmap[BPF_REG_0], 63);
> > +		emit_insn(ctx, addid, LOONGARCH_GPR_T2,
> > LOONGARCH_GPR_ZERO, 0x1);
> > +		emit_cond_jmp(ctx, BPF_JEQ, LOONGARCH_GPR_T1,
> > LOONGARCH_GPR_T2, 3);
> 
> Hi, Haoran,
> 
> Just for codegen, we have many ways to avoid branch. Follows is
> a 
> possible way.
> 
> long long val = regmap[BPF_REG_0];
> int shift = 0 < val ? 32 : 0;
> return (val << shift) >> shift;
> 
> slt    t0, zero, BPF_REG_0
> slli.d t0, t0, 5
> sll.d  BPF_REG_0, BPF_REG_0, t0
> sra.d  BPF_REG_0, BPF_REG_0, t0

Thanks, this code is better.

> >   		emit_insn(ctx, addiw, LOONGARCH_GPR_A0,
> > regmap[BPF_REG_0], 0);
> > +		emit_uncond_jmp(ctx, 2);
> > +		move_reg(ctx, LOONGARCH_GPR_A0, regmap[BPF_REG_0]);
> >   		/* Return to the caller */
> >   		emit_insn(ctx, jirl, LOONGARCH_GPR_ZERO,
> > LOONGARCH_GPR_RA, 0);
> >   	} else {


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2025-08-15  5:12 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-08-14  1:34 [PATCH] LoongArch: BPF: Fix incorrect return pointer value in the eBPF program Haoran Jiang
2025-08-14  3:23 ` Huacai Chen
2025-08-14  4:13   ` Vincent Li
2025-08-14  5:40   ` Re:[PATCH] " jianghaoran
2025-08-14 12:59 ` [PATCH] " Jinyang He
2025-08-15  5:11   ` 回复:[PATCH] " jianghaoran

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).