回复:[PATCH] LoongArch: BPF: Fix incorrect return pointer value in the eBPF program

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



在 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@xxxxxxxxxx
> > >
> > ---
> >   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 {





[Index of Archives]     [Linux Samsung SoC]     [Linux Rockchip SoC]     [Linux Actions SoC]     [Linux for Synopsys ARC Processors]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]


  Powered by Linux