bpf.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2] LoongArch: BPF: Fix incorrect return pointer value in the eBPF program
@ 2025-08-15  8:29 Haoran Jiang
  2025-08-19  8:24 ` Daniel Borkmann
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Haoran Jiang @ 2025-08-15  8:29 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, Jinyang He

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: Jinyang He <hejinyang@loongson.cn>
Signed-off-by: Haoran Jiang <jianghaoran@kylinos.cn>

----------
v2:
1,add emit_slt* helpers
2,Use slt/slld/srad instructions to avoid branch
---
 arch/loongarch/include/asm/inst.h |  8 ++++++++
 arch/loongarch/net/bpf_jit.c      | 17 +++++++++++++++--
 2 files changed, 23 insertions(+), 2 deletions(-)

diff --git a/arch/loongarch/include/asm/inst.h b/arch/loongarch/include/asm/inst.h
index 277d2140676b..20f4fc745bea 100644
--- a/arch/loongarch/include/asm/inst.h
+++ b/arch/loongarch/include/asm/inst.h
@@ -92,6 +92,8 @@ enum reg2i6_op {
 };
 
 enum reg2i12_op {
+	slti_op         = 0x08,
+	sltui_op        = 0x09,
 	addiw_op	= 0x0a,
 	addid_op	= 0x0b,
 	lu52id_op	= 0x0c,
@@ -148,6 +150,8 @@ enum reg3_op {
 	addd_op		= 0x21,
 	subw_op		= 0x22,
 	subd_op		= 0x23,
+	slt_op          = 0x24,
+	sltu_op         = 0x25,
 	nor_op		= 0x28,
 	and_op		= 0x29,
 	or_op		= 0x2a,
@@ -629,6 +633,8 @@ static inline void emit_##NAME(union loongarch_instruction *insn,	\
 	insn->reg2i12_format.rj = rj;					\
 }
 
+DEF_EMIT_REG2I12_FORMAT(slti, slti_op)
+DEF_EMIT_REG2I12_FORMAT(sltui, sltui_op)
 DEF_EMIT_REG2I12_FORMAT(addiw, addiw_op)
 DEF_EMIT_REG2I12_FORMAT(addid, addid_op)
 DEF_EMIT_REG2I12_FORMAT(lu52id, lu52id_op)
@@ -729,6 +735,8 @@ static inline void emit_##NAME(union loongarch_instruction *insn,	\
 DEF_EMIT_REG3_FORMAT(addw, addw_op)
 DEF_EMIT_REG3_FORMAT(addd, addd_op)
 DEF_EMIT_REG3_FORMAT(subd, subd_op)
+DEF_EMIT_REG3_FORMAT(slt, slt_op)
+DEF_EMIT_REG3_FORMAT(sltu, sltu_op)
 DEF_EMIT_REG3_FORMAT(muld, muld_op)
 DEF_EMIT_REG3_FORMAT(divd, divd_op)
 DEF_EMIT_REG3_FORMAT(modd, modd_op)
diff --git a/arch/loongarch/net/bpf_jit.c b/arch/loongarch/net/bpf_jit.c
index abfdb6bb5c38..50067be79c4f 100644
--- a/arch/loongarch/net/bpf_jit.c
+++ b/arch/loongarch/net/bpf_jit.c
@@ -229,8 +229,21 @@ 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 */
-		emit_insn(ctx, addiw, LOONGARCH_GPR_A0, regmap[BPF_REG_0], 0);
+		/*
+		 *  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.
+
+		 *  long long val = regmap[BPF_REG_0];
+		 *  int shift = 0 < val ? 32 : 0;
+		 *  return (val << shift) >> shift;
+		 */
+		move_reg(ctx, LOONGARCH_GPR_A0, regmap[BPF_REG_0]);
+		emit_insn(ctx, slt, LOONGARCH_GPR_T0, LOONGARCH_GPR_ZERO, LOONGARCH_GPR_A0);
+		emit_insn(ctx, sllid, LOONGARCH_GPR_T0, LOONGARCH_GPR_T0, 5);
+		emit_insn(ctx, slld, LOONGARCH_GPR_A0, LOONGARCH_GPR_A0, LOONGARCH_GPR_T0);
+		emit_insn(ctx, srad, LOONGARCH_GPR_A0, LOONGARCH_GPR_A0, LOONGARCH_GPR_T0);
+
 		/* 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 v2] LoongArch: BPF: Fix incorrect return pointer value in the eBPF program
  2025-08-15  8:29 [PATCH v2] LoongArch: BPF: Fix incorrect return pointer value in the eBPF program Haoran Jiang
@ 2025-08-19  8:24 ` Daniel Borkmann
  2025-08-19  9:06   ` Huacai Chen
  2025-08-19 12:32 ` Hengqi Chen
  2025-08-20  4:23 ` Tiezhu Yang
  2 siblings, 1 reply; 6+ messages in thread
From: Daniel Borkmann @ 2025-08-19  8:24 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, ast, Jinyang He

On 8/15/25 10:29 AM, 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: Jinyang He <hejinyang@loongson.cn>
> Signed-off-by: Haoran Jiang <jianghaoran@kylinos.cn>

Huacai, are you routing the fix or want us to route via bpf tree?

Thanks,
Daniel

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

* Re: [PATCH v2] LoongArch: BPF: Fix incorrect return pointer value in the eBPF program
  2025-08-19  8:24 ` Daniel Borkmann
@ 2025-08-19  9:06   ` Huacai Chen
  2025-08-19 13:50     ` Tiezhu Yang
  0 siblings, 1 reply; 6+ messages in thread
From: Huacai Chen @ 2025-08-19  9:06 UTC (permalink / raw)
  To: Daniel Borkmann
  Cc: Haoran Jiang, loongarch, bpf, kernel, hengqi.chen, yangtiezhu,
	jolsa, haoluo, sdf, kpsingh, john.fastabend, yonghong.song, song,
	eddyz87, martin.lau, andrii, ast, Jinyang He

On Tue, Aug 19, 2025 at 4:24 PM Daniel Borkmann <daniel@iogearbox.net> wrote:
>
> On 8/15/25 10:29 AM, 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: Jinyang He <hejinyang@loongson.cn>
> > Signed-off-by: Haoran Jiang <jianghaoran@kylinos.cn>
>
> Huacai, are you routing the fix or want us to route via bpf tree?
I will take it, but I'm waiting Tiezhu's Ack now.

Huacai

>
> Thanks,
> Daniel

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

* Re: [PATCH v2] LoongArch: BPF: Fix incorrect return pointer value in the eBPF program
  2025-08-15  8:29 [PATCH v2] LoongArch: BPF: Fix incorrect return pointer value in the eBPF program Haoran Jiang
  2025-08-19  8:24 ` Daniel Borkmann
@ 2025-08-19 12:32 ` Hengqi Chen
  2025-08-20  4:23 ` Tiezhu Yang
  2 siblings, 0 replies; 6+ messages in thread
From: Hengqi Chen @ 2025-08-19 12:32 UTC (permalink / raw)
  To: Haoran Jiang
  Cc: loongarch, bpf, kernel, chenhuacai, yangtiezhu, jolsa, haoluo,
	sdf, kpsingh, john.fastabend, yonghong.song, song, eddyz87,
	martin.lau, andrii, daniel, ast, Jinyang He

On Fri, Aug 15, 2025 at 4:30 PM 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: Jinyang He <hejinyang@loongson.cn>
> Signed-off-by: Haoran Jiang <jianghaoran@kylinos.cn>
>
> ----------
> v2:
> 1,add emit_slt* helpers
> 2,Use slt/slld/srad instructions to avoid branch
> ---
>  arch/loongarch/include/asm/inst.h |  8 ++++++++
>  arch/loongarch/net/bpf_jit.c      | 17 +++++++++++++++--
>  2 files changed, 23 insertions(+), 2 deletions(-)
>
> diff --git a/arch/loongarch/include/asm/inst.h b/arch/loongarch/include/asm/inst.h
> index 277d2140676b..20f4fc745bea 100644
> --- a/arch/loongarch/include/asm/inst.h
> +++ b/arch/loongarch/include/asm/inst.h
> @@ -92,6 +92,8 @@ enum reg2i6_op {
>  };
>
>  enum reg2i12_op {
> +       slti_op         = 0x08,
> +       sltui_op        = 0x09,
>         addiw_op        = 0x0a,
>         addid_op        = 0x0b,
>         lu52id_op       = 0x0c,
> @@ -148,6 +150,8 @@ enum reg3_op {
>         addd_op         = 0x21,
>         subw_op         = 0x22,
>         subd_op         = 0x23,
> +       slt_op          = 0x24,
> +       sltu_op         = 0x25,
>         nor_op          = 0x28,
>         and_op          = 0x29,
>         or_op           = 0x2a,
> @@ -629,6 +633,8 @@ static inline void emit_##NAME(union loongarch_instruction *insn,   \
>         insn->reg2i12_format.rj = rj;                                   \
>  }
>
> +DEF_EMIT_REG2I12_FORMAT(slti, slti_op)
> +DEF_EMIT_REG2I12_FORMAT(sltui, sltui_op)
>  DEF_EMIT_REG2I12_FORMAT(addiw, addiw_op)
>  DEF_EMIT_REG2I12_FORMAT(addid, addid_op)
>  DEF_EMIT_REG2I12_FORMAT(lu52id, lu52id_op)
> @@ -729,6 +735,8 @@ static inline void emit_##NAME(union loongarch_instruction *insn,   \
>  DEF_EMIT_REG3_FORMAT(addw, addw_op)
>  DEF_EMIT_REG3_FORMAT(addd, addd_op)
>  DEF_EMIT_REG3_FORMAT(subd, subd_op)
> +DEF_EMIT_REG3_FORMAT(slt, slt_op)
> +DEF_EMIT_REG3_FORMAT(sltu, sltu_op)
>  DEF_EMIT_REG3_FORMAT(muld, muld_op)
>  DEF_EMIT_REG3_FORMAT(divd, divd_op)
>  DEF_EMIT_REG3_FORMAT(modd, modd_op)
> diff --git a/arch/loongarch/net/bpf_jit.c b/arch/loongarch/net/bpf_jit.c
> index abfdb6bb5c38..50067be79c4f 100644
> --- a/arch/loongarch/net/bpf_jit.c
> +++ b/arch/loongarch/net/bpf_jit.c
> @@ -229,8 +229,21 @@ 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 */
> -               emit_insn(ctx, addiw, LOONGARCH_GPR_A0, regmap[BPF_REG_0], 0);
> +               /*
> +                *  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.
> +
> +                *  long long val = regmap[BPF_REG_0];
> +                *  int shift = 0 < val ? 32 : 0;
> +                *  return (val << shift) >> shift;
> +                */
> +               move_reg(ctx, LOONGARCH_GPR_A0, regmap[BPF_REG_0]);
> +               emit_insn(ctx, slt, LOONGARCH_GPR_T0, LOONGARCH_GPR_ZERO, LOONGARCH_GPR_A0);
> +               emit_insn(ctx, sllid, LOONGARCH_GPR_T0, LOONGARCH_GPR_T0, 5);
> +               emit_insn(ctx, slld, LOONGARCH_GPR_A0, LOONGARCH_GPR_A0, LOONGARCH_GPR_T0);
> +               emit_insn(ctx, srad, LOONGARCH_GPR_A0, LOONGARCH_GPR_A0, LOONGARCH_GPR_T0);
> +

This change seems wrong to me. Need further investigation.

>                 /* 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 v2] LoongArch: BPF: Fix incorrect return pointer value in the eBPF program
  2025-08-19  9:06   ` Huacai Chen
@ 2025-08-19 13:50     ` Tiezhu Yang
  0 siblings, 0 replies; 6+ messages in thread
From: Tiezhu Yang @ 2025-08-19 13:50 UTC (permalink / raw)
  To: Huacai Chen, Daniel Borkmann
  Cc: Haoran Jiang, loongarch, bpf, kernel, hengqi.chen, jolsa, haoluo,
	sdf, kpsingh, john.fastabend, yonghong.song, song, eddyz87,
	martin.lau, andrii, ast, Jinyang He

On 2025/8/19 下午5:06, Huacai Chen wrote:
> On Tue, Aug 19, 2025 at 4:24 PM Daniel Borkmann <daniel@iogearbox.net> wrote:
>>
>> On 8/15/25 10:29 AM, 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.

...

>> Huacai, are you routing the fix or want us to route via bpf tree?
> I will take it, but I'm waiting Tiezhu's Ack now.

This version has problem, with this patch, many test cases failed
when executing "sudo modprobe test_bpf", the root cause is related
with the instruction "sra.d", I will give a detail reply tomorrow.

Thanks,
Tiezhu


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

* Re: [PATCH v2] LoongArch: BPF: Fix incorrect return pointer value in the eBPF program
  2025-08-15  8:29 [PATCH v2] LoongArch: BPF: Fix incorrect return pointer value in the eBPF program Haoran Jiang
  2025-08-19  8:24 ` Daniel Borkmann
  2025-08-19 12:32 ` Hengqi Chen
@ 2025-08-20  4:23 ` Tiezhu Yang
  2 siblings, 0 replies; 6+ messages in thread
From: Tiezhu Yang @ 2025-08-20  4:23 UTC (permalink / raw)
  To: Haoran Jiang, loongarch
  Cc: bpf, kernel, chenhuacai, hengqi.chen, jolsa, haoluo, sdf, kpsingh,
	john.fastabend, yonghong.song, song, eddyz87, martin.lau, andrii,
	daniel, ast, Jinyang He

On 2025/8/15 下午4:29, 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.

With this v2 patch and the v1 patch:

$ sudo modprobe test_bpf
modprobe: ERROR: could not insert 'test_bpf': Invalid argument
$ dmesg -t | grep Summary
test_bpf: Summary: 1039 PASSED, 14 FAILED, [1041/1041 JIT'ed]

Without the patches, all of the tests passed.
This is related with the previous fix:
LoongArch: BPF: Sign-extend return values
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=73c359d1d356

> diff --git a/arch/loongarch/include/asm/inst.h b/arch/loongarch/include/asm/inst.h
> index 277d2140676b..20f4fc745bea 100644
> --- a/arch/loongarch/include/asm/inst.h
> +++ b/arch/loongarch/include/asm/inst.h
> @@ -92,6 +92,8 @@ enum reg2i6_op {
>   };
>   
>   enum reg2i12_op {
> +	slti_op         = 0x08,
> +	sltui_op        = 0x09,
>   	addiw_op	= 0x0a,
>   	addid_op	= 0x0b,
>   	lu52id_op	= 0x0c,
> @@ -148,6 +150,8 @@ enum reg3_op {
>   	addd_op		= 0x21,
>   	subw_op		= 0x22,
>   	subd_op		= 0x23,
> +	slt_op          = 0x24,
> +	sltu_op         = 0x25,

Only slt is used in bpf_jit.c,
slti, sltui and sltu can be removed
because they are not used in this patch.

> diff --git a/arch/loongarch/net/bpf_jit.c b/arch/loongarch/net/bpf_jit.c
> index abfdb6bb5c38..50067be79c4f 100644
> --- a/arch/loongarch/net/bpf_jit.c
> +++ b/arch/loongarch/net/bpf_jit.c
> @@ -229,8 +229,21 @@ 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 */
> -		emit_insn(ctx, addiw, LOONGARCH_GPR_A0, regmap[BPF_REG_0], 0);
> +		/*
> +		 *  Set return value

It is better to update the comment:

The return value is either a unsigned 32-bit value or a unsigned 64-bit 
address.

> +		 *  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.
> +
> +		 *  long long val = regmap[BPF_REG_0];
> +		 *  int shift = 0 < val ? 32 : 0;
> +		 *  return (val << shift) >> shift;
> +		 */
> +		move_reg(ctx, LOONGARCH_GPR_A0, regmap[BPF_REG_0]);
> +		emit_insn(ctx, slt, LOONGARCH_GPR_T0, LOONGARCH_GPR_ZERO, LOONGARCH_GPR_A0);

LOONGARCH_GPR_T0 may be not the expected value 1 if the return value
is a unsigned 32-bit value.

A quick way to verify this issue:

Add "move_imm(ctx, LOONGARCH_GPR_T0, 1, true)" after "slt" to force
LOONGARCH_GPR_T0 as 1 can fix the above failed tests.

The initial conclusion is that if the 64th bit in regmap[BPF_REG_0]
is 1, it can not recognize the value in regmap[BPF_REG_0] must be a
kernel-space address, it may be a unsigned 32-bit return value, for
this case just use addi.w to extend bit 31 into bits 63 through 32
of a5 to a0.

Thanks,
Tiezhu


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

end of thread, other threads:[~2025-08-20  4:23 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-08-15  8:29 [PATCH v2] LoongArch: BPF: Fix incorrect return pointer value in the eBPF program Haoran Jiang
2025-08-19  8:24 ` Daniel Borkmann
2025-08-19  9:06   ` Huacai Chen
2025-08-19 13:50     ` Tiezhu Yang
2025-08-19 12:32 ` Hengqi Chen
2025-08-20  4:23 ` Tiezhu Yang

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