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

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

 



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@xxxxxxxxxxx>
Signed-off-by: Haoran Jiang <jianghaoran@xxxxxxxxxx>

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





[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