Le 16/01/2025 à 09:45, pangliyuan a écrit :
[Vous ne recevez pas souvent de courriers de pangliyu...@huawei.com. Découvrez
pourquoi ceci est important à https://aka.ms/LearnAboutSenderIdentification ]
When CONFIG_STACKPROTECTOR_STRONG is enabled and FTRACE is disabled on
powerpc64, repeatedly triggering the kprobe process may cause stack check
failures and panic.
Case:
There is a kprobe(do nothing in handler) attached to the "shmem_get_inode",
and a process A is creating file on tmpfs.
CPU0
A |r13 = paca_ptrs[0], paca_ptrs[0]->canary=A->stack_canary
|touch a file on tmpfs
|shmem_mknod():
| load A's canary through r13 and stored it in A's stack
| shmem_get_inode():
| enter kprobe first
| optinsn_slot():
| stored r13 (paca_ptrs[0]) in stack
......
==> schedule, B run on CPU0, A run on CPU1
CPU0
B |r13 = paca_ptrs[0], paca_ptrs[0]->canary=B->stack_canary
|do something...
CPU1
A | r13 = paca_ptrs[1], paca_ptrs[1]->canary=A->stack_canary
| about to leave 'optinsn_slot', restore r13 from A's stack
| r13 = paca_ptrs[0], paca_ptrs[0]->canary=B->stack_canary
| leave optinsn_slot, back to shmem_get_inode
| leave shmem_get_inode, back to shmem_mknod
| do canary check in shmem_mknod, but canary stored in A's stack (A's
canary) doesn't match the canary loaded through r13 (B's canary),
so panic
When A(on CPU0) entring optinsn_slot, it stored r13(paca_ptrs[0]) in stack,
then A is scheduled to CPU1 and restore r13 from A's stack when leaving
'optinsn_slot'. Now A is running on CPU1 but r13 point to CPU0's
paca_ptrs[0], at this time paca_ptrs[0]->__current points to another
process B, which cause A use B's canary to do stack check and panic.
This can be simply fixed by not saving and restoring the r13 register,
because on powerpc64, r13 is a special register that reserved to point
to the current process, no need to restore the outdated r13 if scheduled
had happened.
Does r13 really points to current process ? I thought it was pointing to
PACA which is a structure attached to a given CPU not a given process.
By the way, don't we have the same problem on powerpc32 with register r2 ?
Fixes: 51c9c0843993 ("powerpc/kprobes: Implement Optprobes")
Signed-off-by: pangliyuan <pangliyu...@huawei.com>
---
arch/powerpc/kernel/optprobes_head.S | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/arch/powerpc/kernel/optprobes_head.S
b/arch/powerpc/kernel/optprobes_head.S
index 35932f45fb4e..bf0d77e62ba8 100644
--- a/arch/powerpc/kernel/optprobes_head.S
+++ b/arch/powerpc/kernel/optprobes_head.S
@@ -10,12 +10,12 @@
#include <asm/asm-offsets.h>
#ifdef CONFIG_PPC64
-#define SAVE_30GPRS(base) SAVE_GPRS(2, 31, base)
-#define REST_30GPRS(base) REST_GPRS(2, 31, base)
+#define SAVE_NEEDED_GPRS(base) SAVE_GPRS(2, 12, base); SAVE_GPRS(14, 31, base)
+#define REST_NEEDED_GPRS(base) REST_GPRS(2, 12, base); REST_GPRS(14, 31, base)
This macro name seems a bit sketchy, as far as I understand r0 and r1
also need to be saved/restored allthough they are not handled by this macro.
#define TEMPLATE_FOR_IMM_LOAD_INSNS nop; nop; nop; nop; nop
#else
-#define SAVE_30GPRS(base) stmw r2, GPR2(base)
-#define REST_30GPRS(base) lmw r2, GPR2(base)
+#define SAVE_NEEDED_GPRS(base) stmw r2, GPR2(base)
+#define REST_NEEDED_GPRS(base) lmw r2, GPR2(base)
#define TEMPLATE_FOR_IMM_LOAD_INSNS nop; nop; nop
#endif
@@ -45,7 +45,7 @@ optprobe_template_entry:
/* Save the previous SP into stack */
addi r0,r1,INT_FRAME_SIZE
PPC_STL r0,GPR1(r1)
- SAVE_30GPRS(r1)
+ SAVE_NEEDED_GPRS(r1)
/* Save SPRS */
mfmsr r5
PPC_STL r5,_MSR(r1)
@@ -123,7 +123,7 @@ optprobe_template_call_emulate:
PPC_LL r5,_CCR(r1)
mtcr r5
REST_GPR(0,r1)
- REST_30GPRS(r1)
+ REST_NEEDED_GPRS(r1)
/* Restore the previous SP */
addi r1,r1,INT_FRAME_SIZE
--
2.37.7
