LoongArch architecture development
 help / color / mirror / Atom feed
* [PATCH 0/2] LoongArch: BPF: Remove fatal signed-extension instructions
@ 2023-12-06  7:38 Hengqi Chen
  2023-12-06  7:38 ` [PATCH 1/2] LoongArch: BPF: Don't sign extend function return value Hengqi Chen
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Hengqi Chen @ 2023-12-06  7:38 UTC (permalink / raw)
  To: loongarch; +Cc: chenhuacai, yangtiezhu, hejinyang, hengqi.chen

As reported on mailing list ([0]), several BPF testcases could
triggered kernel panics like:

  # ./test_progs -t cls_redirect
  Can't find bpf_testmod.ko kernel module: -2
  WARNING! Selftests relying on bpf_testmod.ko will be skipped.
  [   30.938489] CPU 3 Unable to handle kernel paging request at virtual address fffffffffd814de0, era == ffff800002009fb8, ra == ffff800002009f9c
  [   30.939331] Oops[#1]:
  [   30.939513] CPU: 3 PID: 1260 Comm: test_progs Not tainted 6.7.0-rc2-loong-devel-g2f56bb0d2327 #35 a896aca3f4164f09cc346f89f2e09832e07be5f6
  [   30.939732] Hardware name: QEMU QEMU Virtual Machine, BIOS unknown 2/2/2022
  [   30.939901] pc ffff800002009fb8 ra ffff800002009f9c tp 9000000104da4000 sp 9000000104da7ab0
  [   30.940038] a0 fffffffffd814de0 a1 9000000104da7a68 a2 0000000000000000 a3 9000000104da7c10
  [   30.940183] a4 9000000104da7c14 a5 0000000000000002 a6 0000000000000021 a7 00005555904d7f90
  [   30.940321] t0 0000000000000110 t1 0000000000000000 t2 fffffffffd814de0 t3 0004c4b400000000
  [   30.940456] t4 ffffffffffffffff t5 00000000c3f63600 t6 0000000000000000 t7 0000000000000000
  [   30.940590] t8 000000000006d803 u0 0000000000000020 s9 9000000104da7b10 s0 900000010504c200
  [   30.940727] s1 fffffffffd814de0 s2 900000010504c200 s3 9000000104da7c10 s4 9000000104da7ad0
  [   30.940866] s5 0000000000000000 s6 90000000030e65bc s7 9000000104da7b44 s8 90000000044f6fc0
  [   30.941015]    ra: ffff800002009f9c bpf_prog_846803e5ae81417f_cls_redirect+0xa0/0x590
  [   30.941535]   ERA: ffff800002009fb8 bpf_prog_846803e5ae81417f_cls_redirect+0xbc/0x590
  [   30.941696]  CRMD: 000000b0 (PLV0 -IE -DA +PG DACF=CC DACM=CC -WE)
  [   30.942224]  PRMD: 00000004 (PPLV0 +PIE -PWE)
  [   30.942330]  EUEN: 00000003 (+FPE +SXE -ASXE -BTE)
  [   30.942453]  ECFG: 00071c1c (LIE=2-4,10-12 VS=7)
  [   30.942612] ESTAT: 00010000 [PIL] (IS= ECode=1 EsubCode=0)
  [   30.942764]  BADV: fffffffffd814de0
  [   30.942854]  PRID: 0014c010 (Loongson-64bit, Loongson-3A5000)
  [   30.942974] Modules linked in:
  [   30.943078] Process test_progs (pid: 1260, threadinfo=00000000ce303226, task=000000007d10bb76)
  [   30.943306] Stack : 900000010a064000 90000000044f6fc0 9000000104da7b48 0000000000000000
  [   30.943495]         0000000000000000 9000000104da7c14 9000000104da7c10 900000010504c200
  [   30.943626]         0000000000000001 ffff80001b88c000 9000000104da7b70 90000000030e6668
  [   30.943785]         0000000000000000 9000000104da7b58 ffff80001b88c048 9000000003d05000
  [   30.943936]         900000000303ac88 0000000000000000 0000000000000000 9000000104da7b70
  [   30.944091]         0000000000000000 0000000000000001 0000000731eeab00 0000000000000000
  [   30.944245]         ffff80001b88c000 0000000000000000 0000000000000000 54b99959429f83b8
  [   30.944402]         ffff80001b88c000 90000000044f6fc0 9000000101d70000 ffff80001b88c000
  [   30.944538]         000000000000005a 900000010504c200 900000010a064000 900000010a067000
  [   30.944697]         9000000104da7d88 0000000000000000 9000000003d05000 90000000030e794c
  [   30.944852]         ...
  [   30.944924] Call Trace:
  [   30.945120] [<ffff800002009fb8>] bpf_prog_846803e5ae81417f_cls_redirect+0xbc/0x590
  [   30.945650] [<90000000030e6668>] bpf_test_run+0x1ec/0x2f8
  [   30.945958] [<90000000030e794c>] bpf_prog_test_run_skb+0x31c/0x684
  [   30.946065] [<90000000026d4f68>] __sys_bpf+0x678/0x2724
  [   30.946159] [<90000000026d7288>] sys_bpf+0x20/0x2c
  [   30.946253] [<90000000032dd224>] do_syscall+0x7c/0x94
  [   30.946343] [<9000000002541c5c>] handle_syscall+0xbc/0x158
  [   30.946492]
  [   30.946549] Code: 0015030e  5c0009c0  5001d000 <28c00304> 02c00484  29c00304  00150009  2a42d2e4  0280200d
  [   30.946793]
  [   30.946971] ---[ end trace 0000000000000000 ]---
  [   32.093225] Kernel panic - not syncing: Fatal exception in interrupt
  [   32.093526] Kernel relocated by 0x2320000
  [   32.093630]  .text @ 0x9000000002520000
  [   32.093725]  .data @ 0x9000000003400000
  [   32.093792]  .bss  @ 0x9000000004413200
  [   34.971998] ---[ end Kernel panic - not syncing: Fatal exception in interrupt ]---

Tracking this down reveals two different code paths lead to
these panics.

1. Signed extend function return value

  Take the `cls_redirect` test as an example, when subprog mode
  is enabled, we have:

    cls_redirect()
      -> get_global_metrics() returns pcpu ptr 0xfffffefffc00b480

  The pointer is later signed-extended to 0xfffffffffc00b480 at
  `BPF_JMP | BPF_EXIT`. During BPF prog run, this triggers unhandled
  page fault and a kernel panic.

  The `verifier_map_ptr_mixing` test is another victim of this
  signed-extension.

  BPF verifier ensures all BPF progs loaded run to completion, so
  every BPF prog has at least one `exit` instruction. Why do most of
  them survive this signed-extension? That's because most BPF prog
  types (if not all) return small non-negative integers, like TC or XDP.
  Some prog types (like tracing) even ignore the return value.
  In the above cases, we use subprogs, and subprogs return pointer
  values, so bad things happened.

2. Signed extend memory load operand

  The `cgrp_local_storage` test triggers panics when doing memory
  load (see `cgrp_ls_tp_btf.c` for details):

    ptr = bpf_cgrp_storage_get(&map_a, task->cgroups->dfl_cgrp, 0,
                               BPF_LOCAL_STORAGE_GET_F_CREATE);

  The expression `task->cgroups->dfl_cgrp` involves two memory load.
  Since the field offset fits in imm12 or imm14, we use ldd or ldptrd
  to perform memory load. But the ldd and ldptrd instructions both
  have the side effect that it will signed-extended the imm operand.
  Finally, we got the wrong addresses and result in kernel panics.

Let's get rid of those fatal signed-extension instrutions to fix
these issues.
  - For the first case, don't sext on return values like other arches.
  - For the second case, we fallback to use a generic ldxd instruction.

  [0]: https://lore.kernel.org/loongarch/CAEyhmHT6UOryPZi7eEfw01Y_CjAqxy8NM6eKkdvdojJqB9P7eQ@mail.gmail.com/

Hengqi Chen (2):
  LoongArch: BPF: Don't sign extend function return value
  LoongArch: BPF: Don't sign extend memory load operand

 arch/loongarch/net/bpf_jit.c | 12 ++----------
 1 file changed, 2 insertions(+), 10 deletions(-)

--
2.34.1

^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-12-08  0:48 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-12-06  7:38 [PATCH 0/2] LoongArch: BPF: Remove fatal signed-extension instructions Hengqi Chen
2023-12-06  7:38 ` [PATCH 1/2] LoongArch: BPF: Don't sign extend function return value Hengqi Chen
2023-12-06  7:38 ` [PATCH 2/2] LoongArch: BPF: Don't sign extend memory load operand Hengqi Chen
2023-12-07 10:05 ` [PATCH 0/2] LoongArch: BPF: Remove fatal signed-extension instructions Huacai Chen
2023-12-07 14:49   ` Hengqi Chen
2023-12-08  0:48     ` Huacai Chen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox