| Message ID | 20220523073732.296247-1-imagedong@tencent.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | BPF |
| Headers | show |
| Series | bpf: fix probe read error in ___bpf_prog_run() | expand |
On 5/23/22 9:37 AM, menglong8.dong@gmail.com wrote: > From: Menglong Dong <imagedong@tencent.com> > > I think there is something wrong with BPF_PROBE_MEM in ___bpf_prog_run() > in big-endian machine. Let's make a test and see what will happen if we > want to load a 'u16' with BPF_PROBE_MEM. > > Let's make the src value '0x0001', the value of dest register will become > 0x0001000000000000, as the value will be loaded to the first 2 byte of > DST with following code: > > bpf_probe_read_kernel(&DST, SIZE, (const void *)(long) (SRC + insn->off)); > > Obviously, the value in DST is not correct. In fact, we can compare > BPF_PROBE_MEM with LDX_MEM_H: > > DST = *(SIZE *)(unsigned long) (SRC + insn->off); > > If the memory load is done by LDX_MEM_H, the value in DST will be 0x1 now. > > And I think this error results in the test case 'test_bpf_sk_storage_map' > failing: > > test_bpf_sk_storage_map:PASS:bpf_iter_bpf_sk_storage_map__open_and_load 0 nsec > test_bpf_sk_storage_map:PASS:socket 0 nsec > test_bpf_sk_storage_map:PASS:map_update 0 nsec > test_bpf_sk_storage_map:PASS:socket 0 nsec > test_bpf_sk_storage_map:PASS:map_update 0 nsec > test_bpf_sk_storage_map:PASS:socket 0 nsec > test_bpf_sk_storage_map:PASS:map_update 0 nsec > test_bpf_sk_storage_map:PASS:attach_iter 0 nsec > test_bpf_sk_storage_map:PASS:create_iter 0 nsec > test_bpf_sk_storage_map:PASS:read 0 nsec > test_bpf_sk_storage_map:FAIL:ipv6_sk_count got 0 expected 3 > $10/26 bpf_iter/bpf_sk_storage_map:FAIL > > The code of the test case is simply, it will load sk->sk_family to the > register with BPF_PROBE_MEM and check if it is AF_INET6. With this patch, > now the test case 'bpf_iter' can pass: > > $10 bpf_iter:OK > > Reviewed-by: Jiang Biao <benbjiang@tencent.com> > Reviewed-by: Hao Peng <flyingpeng@tencent.com> > Signed-off-by: Menglong Dong <imagedong@tencent.com> > --- > kernel/bpf/core.c | 11 ++++++----- > 1 file changed, 6 insertions(+), 5 deletions(-) > > diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c > index 13e9dbeeedf3..09e3f374739a 100644 > --- a/kernel/bpf/core.c > +++ b/kernel/bpf/core.c > @@ -1945,14 +1945,15 @@ static u64 ___bpf_prog_run(u64 *regs, const struct bpf_insn *insn) > LDST(W, u32) > LDST(DW, u64) > #undef LDST > -#define LDX_PROBE(SIZEOP, SIZE) \ > +#define LDX_PROBE(SIZEOP, SIZE, TYPE) \ > LDX_PROBE_MEM_##SIZEOP: \ > bpf_probe_read_kernel(&DST, SIZE, (const void *)(long) (SRC + insn->off)); \ > + DST = *((TYPE *)&DST); \ > CONT; > - LDX_PROBE(B, 1) > - LDX_PROBE(H, 2) > - LDX_PROBE(W, 4) > - LDX_PROBE(DW, 8) > + LDX_PROBE(B, 1, u8) > + LDX_PROBE(H, 2, u16) > + LDX_PROBE(W, 4, u32) > + LDX_PROBE(DW, 8, u64) Completely uncompiled, but maybe just fold it into LDST instead: diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c index 9cc91f0f3115..fc5c29243739 100644 --- a/kernel/bpf/core.c +++ b/kernel/bpf/core.c @@ -1948,6 +1948,11 @@ static u64 ___bpf_prog_run(u64 *regs, const struct bpf_insn *insn) CONT; \ LDX_MEM_##SIZEOP: \ DST = *(SIZE *)(unsigned long) (SRC + insn->off); \ + CONT; \ + LDX_PROBE_MEM_##SIZEOP: \ + bpf_probe_read_kernel(&DST, sizeof(SIZE), \ + (const void *)(long)(SRC + insn->off)); \ + DST = *((SIZE *)&DST); \ CONT; LDST(B, u8) @@ -1955,15 +1960,6 @@ static u64 ___bpf_prog_run(u64 *regs, const struct bpf_insn *insn) LDST(W, u32) LDST(DW, u64) #undef LDST -#define LDX_PROBE(SIZEOP, SIZE) \ - LDX_PROBE_MEM_##SIZEOP: \ - bpf_probe_read_kernel(&DST, SIZE, (const void *)(long) (SRC + insn->off)); \ - CONT; - LDX_PROBE(B, 1) - LDX_PROBE(H, 2) - LDX_PROBE(W, 4) - LDX_PROBE(DW, 8) -#undef LDX_PROBE Thanks, Daniel
diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c index 13e9dbeeedf3..09e3f374739a 100644 --- a/kernel/bpf/core.c +++ b/kernel/bpf/core.c @@ -1945,14 +1945,15 @@ static u64 ___bpf_prog_run(u64 *regs, const struct bpf_insn *insn) LDST(W, u32) LDST(DW, u64) #undef LDST -#define LDX_PROBE(SIZEOP, SIZE) \ +#define LDX_PROBE(SIZEOP, SIZE, TYPE) \ LDX_PROBE_MEM_##SIZEOP: \ bpf_probe_read_kernel(&DST, SIZE, (const void *)(long) (SRC + insn->off)); \ + DST = *((TYPE *)&DST); \ CONT; - LDX_PROBE(B, 1) - LDX_PROBE(H, 2) - LDX_PROBE(W, 4) - LDX_PROBE(DW, 8) + LDX_PROBE(B, 1, u8) + LDX_PROBE(H, 2, u16) + LDX_PROBE(W, 4, u32) + LDX_PROBE(DW, 8, u64) #undef LDX_PROBE #define ATOMIC_ALU_OP(BOP, KOP) \