* [PATCH 0/2] Modify some code about kprobe
@ 2022-05-05 12:19 Tiezhu Yang
2022-05-05 12:19 ` [PATCH 1/2] selftests/ftrace: Save kprobe_events to test log Tiezhu Yang
2022-05-05 12:19 ` [PATCH 2/2] MIPS: Use NOKPROBE_SYMBOL() instead of __kprobes annotation Tiezhu Yang
0 siblings, 2 replies; 6+ messages in thread
From: Tiezhu Yang @ 2022-05-05 12:19 UTC (permalink / raw)
To: Thomas Bogendoerfer, Masami Hiramatsu, Steven Rostedt, Shuah Khan
Cc: Xuefeng Li, linux-mips, linux-kselftest, linux-kernel
This patch series is preparation to fix the problems when execute
the multiple_kprobes.tc selftest on mips, some more work needs to
be done.
Tiezhu Yang (2):
selftests/ftrace: Save kprobe_events to test log
MIPS: Use NOKPROBE_SYMBOL() instead of __kprobes annotation
arch/mips/kernel/kprobes.c | 45 +++++++++++++++-------
arch/mips/mm/fault.c | 6 ++-
.../ftrace/test.d/kprobe/multiple_kprobes.tc | 2 +
3 files changed, 38 insertions(+), 15 deletions(-)
--
2.1.0
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/2] selftests/ftrace: Save kprobe_events to test log
2022-05-05 12:19 [PATCH 0/2] Modify some code about kprobe Tiezhu Yang
@ 2022-05-05 12:19 ` Tiezhu Yang
2022-05-06 1:46 ` Masami Hiramatsu
2022-05-05 12:19 ` [PATCH 2/2] MIPS: Use NOKPROBE_SYMBOL() instead of __kprobes annotation Tiezhu Yang
1 sibling, 1 reply; 6+ messages in thread
From: Tiezhu Yang @ 2022-05-05 12:19 UTC (permalink / raw)
To: Thomas Bogendoerfer, Masami Hiramatsu, Steven Rostedt, Shuah Khan
Cc: Xuefeng Li, linux-mips, linux-kselftest, linux-kernel
It may lead to kernel panic when execute the following testcase on mips:
# ./ftracetest test.d/kprobe/multiple_kprobes.tc
A preliminary analysis shows that the issue is related with
echo 1 > events/kprobes/enable
after add the 256 probe points.
In order to find the root cause, I want to verify which probe point has
problem, so it is necessary to save kprobe_events to test log.
With this patch, we can get the 256 probe points in the test log through
the following command:
# ./ftracetest test.d/kprobe/multiple_kprobes.tc -vvv -k
Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
---
tools/testing/selftests/ftrace/test.d/kprobe/multiple_kprobes.tc | 2 ++
1 file changed, 2 insertions(+)
diff --git a/tools/testing/selftests/ftrace/test.d/kprobe/multiple_kprobes.tc b/tools/testing/selftests/ftrace/test.d/kprobe/multiple_kprobes.tc
index 312d237..be754f5 100644
--- a/tools/testing/selftests/ftrace/test.d/kprobe/multiple_kprobes.tc
+++ b/tools/testing/selftests/ftrace/test.d/kprobe/multiple_kprobes.tc
@@ -25,6 +25,8 @@ if [ $L -ne 256 ]; then
exit_fail
fi
+cat kprobe_events >> $testlog
+
echo 1 > events/kprobes/enable
echo 0 > events/kprobes/enable
echo > kprobe_events
--
2.1.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/2] MIPS: Use NOKPROBE_SYMBOL() instead of __kprobes annotation
2022-05-05 12:19 [PATCH 0/2] Modify some code about kprobe Tiezhu Yang
2022-05-05 12:19 ` [PATCH 1/2] selftests/ftrace: Save kprobe_events to test log Tiezhu Yang
@ 2022-05-05 12:19 ` Tiezhu Yang
2022-05-06 1:45 ` Masami Hiramatsu
1 sibling, 1 reply; 6+ messages in thread
From: Tiezhu Yang @ 2022-05-05 12:19 UTC (permalink / raw)
To: Thomas Bogendoerfer, Masami Hiramatsu, Steven Rostedt, Shuah Khan
Cc: Xuefeng Li, linux-mips, linux-kselftest, linux-kernel
If define CONFIG_KPROBES, __kprobes annotation forces the whole function
into the ".kprobes.text" section, NOKPROBE_SYMBOL() just stores the given
function address in the "_kprobe_blacklist" section which is introduced
to maintain kprobes blacklist.
Modify the related code to use NOKPROBE_SYMBOL() to protect functions from
kprobes instead of __kprobes annotation under arch/mips.
Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
---
arch/mips/kernel/kprobes.c | 45 ++++++++++++++++++++++++++++++++-------------
arch/mips/mm/fault.c | 6 ++++--
2 files changed, 36 insertions(+), 15 deletions(-)
diff --git a/arch/mips/kernel/kprobes.c b/arch/mips/kernel/kprobes.c
index 6c7f3b1..21f9cec 100644
--- a/arch/mips/kernel/kprobes.c
+++ b/arch/mips/kernel/kprobes.c
@@ -44,10 +44,11 @@ static const union mips_instruction breakpoint2_insn = {
DEFINE_PER_CPU(struct kprobe *, current_kprobe);
DEFINE_PER_CPU(struct kprobe_ctlblk, kprobe_ctlblk);
-static int __kprobes insn_has_delayslot(union mips_instruction insn)
+static int insn_has_delayslot(union mips_instruction insn)
{
return __insn_has_delay_slot(insn);
}
+NOKPROBE_SYMBOL(insn_has_delayslot);
/*
* insn_has_ll_or_sc function checks whether instruction is ll or sc
@@ -56,7 +57,7 @@ static int __kprobes insn_has_delayslot(union mips_instruction insn)
* instructions; cannot do much about breakpoint in the middle of
* ll/sc pair; it is upto user to avoid those places
*/
-static int __kprobes insn_has_ll_or_sc(union mips_instruction insn)
+static int insn_has_ll_or_sc(union mips_instruction insn)
{
int ret = 0;
@@ -72,8 +73,9 @@ static int __kprobes insn_has_ll_or_sc(union mips_instruction insn)
}
return ret;
}
+NOKPROBE_SYMBOL(insn_has_ll_or_sc);
-int __kprobes arch_prepare_kprobe(struct kprobe *p)
+int arch_prepare_kprobe(struct kprobe *p)
{
union mips_instruction insn;
union mips_instruction prev_insn;
@@ -132,26 +134,30 @@ int __kprobes arch_prepare_kprobe(struct kprobe *p)
out:
return ret;
}
+NOKPROBE_SYMBOL(arch_prepare_kprobe);
-void __kprobes arch_arm_kprobe(struct kprobe *p)
+void arch_arm_kprobe(struct kprobe *p)
{
*p->addr = breakpoint_insn;
flush_insn_slot(p);
}
+NOKPROBE_SYMBOL(arch_arm_kprobe);
-void __kprobes arch_disarm_kprobe(struct kprobe *p)
+void arch_disarm_kprobe(struct kprobe *p)
{
*p->addr = p->opcode;
flush_insn_slot(p);
}
+NOKPROBE_SYMBOL(arch_disarm_kprobe);
-void __kprobes arch_remove_kprobe(struct kprobe *p)
+void arch_remove_kprobe(struct kprobe *p)
{
if (p->ainsn.insn) {
free_insn_slot(p->ainsn.insn, 0);
p->ainsn.insn = NULL;
}
}
+NOKPROBE_SYMBOL(arch_remove_kprobe);
static void save_previous_kprobe(struct kprobe_ctlblk *kcb)
{
@@ -161,6 +167,7 @@ static void save_previous_kprobe(struct kprobe_ctlblk *kcb)
kcb->prev_kprobe.saved_SR = kcb->kprobe_saved_SR;
kcb->prev_kprobe.saved_epc = kcb->kprobe_saved_epc;
}
+NOKPROBE_SYMBOL(save_previous_kprobe);
static void restore_previous_kprobe(struct kprobe_ctlblk *kcb)
{
@@ -170,6 +177,7 @@ static void restore_previous_kprobe(struct kprobe_ctlblk *kcb)
kcb->kprobe_saved_SR = kcb->prev_kprobe.saved_SR;
kcb->kprobe_saved_epc = kcb->prev_kprobe.saved_epc;
}
+NOKPROBE_SYMBOL(restore_previous_kprobe);
static void set_current_kprobe(struct kprobe *p, struct pt_regs *regs,
struct kprobe_ctlblk *kcb)
@@ -178,6 +186,7 @@ static void set_current_kprobe(struct kprobe *p, struct pt_regs *regs,
kcb->kprobe_saved_SR = kcb->kprobe_old_SR = (regs->cp0_status & ST0_IE);
kcb->kprobe_saved_epc = regs->cp0_epc;
}
+NOKPROBE_SYMBOL(set_current_kprobe);
/**
* evaluate_branch_instrucion -
@@ -225,6 +234,7 @@ static int evaluate_branch_instruction(struct kprobe *p, struct pt_regs *regs,
return -EFAULT;
}
+NOKPROBE_SYMBOL(evaluate_branch_instruction);
static void prepare_singlestep(struct kprobe *p, struct pt_regs *regs,
struct kprobe_ctlblk *kcb)
@@ -244,6 +254,7 @@ static void prepare_singlestep(struct kprobe *p, struct pt_regs *regs,
}
regs->cp0_epc = (unsigned long)&p->ainsn.insn[0];
}
+NOKPROBE_SYMBOL(prepare_singlestep);
/*
* Called after single-stepping. p->addr is the address of the
@@ -257,7 +268,7 @@ static void prepare_singlestep(struct kprobe *p, struct pt_regs *regs,
* breakpoint trap. In case of branch instructions, the target
* epc to be restored.
*/
-static void __kprobes resume_execution(struct kprobe *p,
+static void resume_execution(struct kprobe *p,
struct pt_regs *regs,
struct kprobe_ctlblk *kcb)
{
@@ -268,8 +279,9 @@ static void __kprobes resume_execution(struct kprobe *p,
regs->cp0_epc = orig_epc + 4;
}
}
+NOKPROBE_SYMBOL(resume_execution);
-static int __kprobes kprobe_handler(struct pt_regs *regs)
+static int kprobe_handler(struct pt_regs *regs)
{
struct kprobe *p;
int ret = 0;
@@ -367,8 +379,9 @@ static int __kprobes kprobe_handler(struct pt_regs *regs)
return ret;
}
+NOKPROBE_SYMBOL(kprobe_handler);
-static inline int post_kprobe_handler(struct pt_regs *regs)
+static int post_kprobe_handler(struct pt_regs *regs)
{
struct kprobe *cur = kprobe_running();
struct kprobe_ctlblk *kcb = get_kprobe_ctlblk();
@@ -396,6 +409,7 @@ static inline int post_kprobe_handler(struct pt_regs *regs)
return 1;
}
+NOKPROBE_SYMBOL(post_kprobe_handler);
int kprobe_fault_handler(struct pt_regs *regs, int trapnr)
{
@@ -411,11 +425,12 @@ int kprobe_fault_handler(struct pt_regs *regs, int trapnr)
}
return 0;
}
+NOKPROBE_SYMBOL(kprobe_fault_handler);
/*
* Wrapper routine for handling exceptions.
*/
-int __kprobes kprobe_exceptions_notify(struct notifier_block *self,
+int kprobe_exceptions_notify(struct notifier_block *self,
unsigned long val, void *data)
{
@@ -446,6 +461,7 @@ int __kprobes kprobe_exceptions_notify(struct notifier_block *self,
}
return ret;
}
+NOKPROBE_SYMBOL(kprobe_exceptions_notify);
/*
* Function return probe trampoline:
@@ -469,7 +485,7 @@ static void __used kretprobe_trampoline_holder(void)
void __kretprobe_trampoline(void);
-void __kprobes arch_prepare_kretprobe(struct kretprobe_instance *ri,
+void arch_prepare_kretprobe(struct kretprobe_instance *ri,
struct pt_regs *regs)
{
ri->ret_addr = (kprobe_opcode_t *) regs->regs[31];
@@ -478,11 +494,12 @@ void __kprobes arch_prepare_kretprobe(struct kretprobe_instance *ri,
/* Replace the return addr with trampoline addr */
regs->regs[31] = (unsigned long)__kretprobe_trampoline;
}
+NOKPROBE_SYMBOL(arch_prepare_kretprobe);
/*
* Called when the probe at kretprobe trampoline is hit
*/
-static int __kprobes trampoline_probe_handler(struct kprobe *p,
+static int trampoline_probe_handler(struct kprobe *p,
struct pt_regs *regs)
{
instruction_pointer(regs) = __kretprobe_trampoline_handler(regs, NULL);
@@ -493,14 +510,16 @@ static int __kprobes trampoline_probe_handler(struct kprobe *p,
*/
return 1;
}
+NOKPROBE_SYMBOL(trampoline_probe_handler);
-int __kprobes arch_trampoline_kprobe(struct kprobe *p)
+int arch_trampoline_kprobe(struct kprobe *p)
{
if (p->addr == (kprobe_opcode_t *)__kretprobe_trampoline)
return 1;
return 0;
}
+NOKPROBE_SYMBOL(arch_trampoline_kprobe);
static struct kprobe trampoline_p = {
.addr = (kprobe_opcode_t *)__kretprobe_trampoline,
diff --git a/arch/mips/mm/fault.c b/arch/mips/mm/fault.c
index 44f9810..b08bc55 100644
--- a/arch/mips/mm/fault.c
+++ b/arch/mips/mm/fault.c
@@ -35,7 +35,7 @@ int show_unhandled_signals = 1;
* and the problem, and then passes it off to one of the appropriate
* routines.
*/
-static void __kprobes __do_page_fault(struct pt_regs *regs, unsigned long write,
+static void __do_page_fault(struct pt_regs *regs, unsigned long write,
unsigned long address)
{
struct vm_area_struct * vma = NULL;
@@ -322,8 +322,9 @@ static void __kprobes __do_page_fault(struct pt_regs *regs, unsigned long write,
}
#endif
}
+NOKPROBE_SYMBOL(__do_page_fault);
-asmlinkage void __kprobes do_page_fault(struct pt_regs *regs,
+asmlinkage void do_page_fault(struct pt_regs *regs,
unsigned long write, unsigned long address)
{
enum ctx_state prev_state;
@@ -332,3 +333,4 @@ asmlinkage void __kprobes do_page_fault(struct pt_regs *regs,
__do_page_fault(regs, write, address);
exception_exit(prev_state);
}
+NOKPROBE_SYMBOL(do_page_fault);
--
2.1.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] MIPS: Use NOKPROBE_SYMBOL() instead of __kprobes annotation
2022-05-05 12:19 ` [PATCH 2/2] MIPS: Use NOKPROBE_SYMBOL() instead of __kprobes annotation Tiezhu Yang
@ 2022-05-06 1:45 ` Masami Hiramatsu
2022-05-06 3:24 ` Tiezhu Yang
0 siblings, 1 reply; 6+ messages in thread
From: Masami Hiramatsu @ 2022-05-06 1:45 UTC (permalink / raw)
To: Tiezhu Yang
Cc: Thomas Bogendoerfer, Steven Rostedt, Shuah Khan, Xuefeng Li,
linux-mips, linux-kselftest, linux-kernel
On Thu, 5 May 2022 20:19:08 +0800
Tiezhu Yang <yangtiezhu@loongson.cn> wrote:
> If define CONFIG_KPROBES, __kprobes annotation forces the whole function
> into the ".kprobes.text" section, NOKPROBE_SYMBOL() just stores the given
> function address in the "_kprobe_blacklist" section which is introduced
> to maintain kprobes blacklist.
>
> Modify the related code to use NOKPROBE_SYMBOL() to protect functions from
> kprobes instead of __kprobes annotation under arch/mips.
So you added some non '__kprobes' annotated functions to NOKPROBE_SYMBOL()
in this patch. Those caused the kernel panic, right? If so, please add such
comment on this description too. Or, split this into 2 patches, one fixes
the kernel panic by adding those functions to NOKPROBE_SYMBOL() and the
other is replacing __kprobes with NOKPROBE_SYMBOL().
Also, could you also find the commit which introduces the kernel panic?
It is worth to backport such fix to stable trees.
Thank you,
>
> Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
> ---
> arch/mips/kernel/kprobes.c | 45 ++++++++++++++++++++++++++++++++-------------
> arch/mips/mm/fault.c | 6 ++++--
> 2 files changed, 36 insertions(+), 15 deletions(-)
>
> diff --git a/arch/mips/kernel/kprobes.c b/arch/mips/kernel/kprobes.c
> index 6c7f3b1..21f9cec 100644
> --- a/arch/mips/kernel/kprobes.c
> +++ b/arch/mips/kernel/kprobes.c
> @@ -44,10 +44,11 @@ static const union mips_instruction breakpoint2_insn = {
> DEFINE_PER_CPU(struct kprobe *, current_kprobe);
> DEFINE_PER_CPU(struct kprobe_ctlblk, kprobe_ctlblk);
>
> -static int __kprobes insn_has_delayslot(union mips_instruction insn)
> +static int insn_has_delayslot(union mips_instruction insn)
> {
> return __insn_has_delay_slot(insn);
> }
> +NOKPROBE_SYMBOL(insn_has_delayslot);
>
> /*
> * insn_has_ll_or_sc function checks whether instruction is ll or sc
> @@ -56,7 +57,7 @@ static int __kprobes insn_has_delayslot(union mips_instruction insn)
> * instructions; cannot do much about breakpoint in the middle of
> * ll/sc pair; it is upto user to avoid those places
> */
> -static int __kprobes insn_has_ll_or_sc(union mips_instruction insn)
> +static int insn_has_ll_or_sc(union mips_instruction insn)
> {
> int ret = 0;
>
> @@ -72,8 +73,9 @@ static int __kprobes insn_has_ll_or_sc(union mips_instruction insn)
> }
> return ret;
> }
> +NOKPROBE_SYMBOL(insn_has_ll_or_sc);
>
> -int __kprobes arch_prepare_kprobe(struct kprobe *p)
> +int arch_prepare_kprobe(struct kprobe *p)
> {
> union mips_instruction insn;
> union mips_instruction prev_insn;
> @@ -132,26 +134,30 @@ int __kprobes arch_prepare_kprobe(struct kprobe *p)
> out:
> return ret;
> }
> +NOKPROBE_SYMBOL(arch_prepare_kprobe);
>
> -void __kprobes arch_arm_kprobe(struct kprobe *p)
> +void arch_arm_kprobe(struct kprobe *p)
> {
> *p->addr = breakpoint_insn;
> flush_insn_slot(p);
> }
> +NOKPROBE_SYMBOL(arch_arm_kprobe);
>
> -void __kprobes arch_disarm_kprobe(struct kprobe *p)
> +void arch_disarm_kprobe(struct kprobe *p)
> {
> *p->addr = p->opcode;
> flush_insn_slot(p);
> }
> +NOKPROBE_SYMBOL(arch_disarm_kprobe);
>
> -void __kprobes arch_remove_kprobe(struct kprobe *p)
> +void arch_remove_kprobe(struct kprobe *p)
> {
> if (p->ainsn.insn) {
> free_insn_slot(p->ainsn.insn, 0);
> p->ainsn.insn = NULL;
> }
> }
> +NOKPROBE_SYMBOL(arch_remove_kprobe);
>
> static void save_previous_kprobe(struct kprobe_ctlblk *kcb)
> {
> @@ -161,6 +167,7 @@ static void save_previous_kprobe(struct kprobe_ctlblk *kcb)
> kcb->prev_kprobe.saved_SR = kcb->kprobe_saved_SR;
> kcb->prev_kprobe.saved_epc = kcb->kprobe_saved_epc;
> }
> +NOKPROBE_SYMBOL(save_previous_kprobe);
>
> static void restore_previous_kprobe(struct kprobe_ctlblk *kcb)
> {
> @@ -170,6 +177,7 @@ static void restore_previous_kprobe(struct kprobe_ctlblk *kcb)
> kcb->kprobe_saved_SR = kcb->prev_kprobe.saved_SR;
> kcb->kprobe_saved_epc = kcb->prev_kprobe.saved_epc;
> }
> +NOKPROBE_SYMBOL(restore_previous_kprobe);
>
> static void set_current_kprobe(struct kprobe *p, struct pt_regs *regs,
> struct kprobe_ctlblk *kcb)
> @@ -178,6 +186,7 @@ static void set_current_kprobe(struct kprobe *p, struct pt_regs *regs,
> kcb->kprobe_saved_SR = kcb->kprobe_old_SR = (regs->cp0_status & ST0_IE);
> kcb->kprobe_saved_epc = regs->cp0_epc;
> }
> +NOKPROBE_SYMBOL(set_current_kprobe);
>
> /**
> * evaluate_branch_instrucion -
> @@ -225,6 +234,7 @@ static int evaluate_branch_instruction(struct kprobe *p, struct pt_regs *regs,
> return -EFAULT;
>
> }
> +NOKPROBE_SYMBOL(evaluate_branch_instruction);
>
> static void prepare_singlestep(struct kprobe *p, struct pt_regs *regs,
> struct kprobe_ctlblk *kcb)
> @@ -244,6 +254,7 @@ static void prepare_singlestep(struct kprobe *p, struct pt_regs *regs,
> }
> regs->cp0_epc = (unsigned long)&p->ainsn.insn[0];
> }
> +NOKPROBE_SYMBOL(prepare_singlestep);
>
> /*
> * Called after single-stepping. p->addr is the address of the
> @@ -257,7 +268,7 @@ static void prepare_singlestep(struct kprobe *p, struct pt_regs *regs,
> * breakpoint trap. In case of branch instructions, the target
> * epc to be restored.
> */
> -static void __kprobes resume_execution(struct kprobe *p,
> +static void resume_execution(struct kprobe *p,
> struct pt_regs *regs,
> struct kprobe_ctlblk *kcb)
> {
> @@ -268,8 +279,9 @@ static void __kprobes resume_execution(struct kprobe *p,
> regs->cp0_epc = orig_epc + 4;
> }
> }
> +NOKPROBE_SYMBOL(resume_execution);
>
> -static int __kprobes kprobe_handler(struct pt_regs *regs)
> +static int kprobe_handler(struct pt_regs *regs)
> {
> struct kprobe *p;
> int ret = 0;
> @@ -367,8 +379,9 @@ static int __kprobes kprobe_handler(struct pt_regs *regs)
> return ret;
>
> }
> +NOKPROBE_SYMBOL(kprobe_handler);
>
> -static inline int post_kprobe_handler(struct pt_regs *regs)
> +static int post_kprobe_handler(struct pt_regs *regs)
> {
> struct kprobe *cur = kprobe_running();
> struct kprobe_ctlblk *kcb = get_kprobe_ctlblk();
> @@ -396,6 +409,7 @@ static inline int post_kprobe_handler(struct pt_regs *regs)
>
> return 1;
> }
> +NOKPROBE_SYMBOL(post_kprobe_handler);
>
> int kprobe_fault_handler(struct pt_regs *regs, int trapnr)
> {
> @@ -411,11 +425,12 @@ int kprobe_fault_handler(struct pt_regs *regs, int trapnr)
> }
> return 0;
> }
> +NOKPROBE_SYMBOL(kprobe_fault_handler);
>
> /*
> * Wrapper routine for handling exceptions.
> */
> -int __kprobes kprobe_exceptions_notify(struct notifier_block *self,
> +int kprobe_exceptions_notify(struct notifier_block *self,
> unsigned long val, void *data)
> {
>
> @@ -446,6 +461,7 @@ int __kprobes kprobe_exceptions_notify(struct notifier_block *self,
> }
> return ret;
> }
> +NOKPROBE_SYMBOL(kprobe_exceptions_notify);
>
> /*
> * Function return probe trampoline:
> @@ -469,7 +485,7 @@ static void __used kretprobe_trampoline_holder(void)
>
> void __kretprobe_trampoline(void);
>
> -void __kprobes arch_prepare_kretprobe(struct kretprobe_instance *ri,
> +void arch_prepare_kretprobe(struct kretprobe_instance *ri,
> struct pt_regs *regs)
> {
> ri->ret_addr = (kprobe_opcode_t *) regs->regs[31];
> @@ -478,11 +494,12 @@ void __kprobes arch_prepare_kretprobe(struct kretprobe_instance *ri,
> /* Replace the return addr with trampoline addr */
> regs->regs[31] = (unsigned long)__kretprobe_trampoline;
> }
> +NOKPROBE_SYMBOL(arch_prepare_kretprobe);
>
> /*
> * Called when the probe at kretprobe trampoline is hit
> */
> -static int __kprobes trampoline_probe_handler(struct kprobe *p,
> +static int trampoline_probe_handler(struct kprobe *p,
> struct pt_regs *regs)
> {
> instruction_pointer(regs) = __kretprobe_trampoline_handler(regs, NULL);
> @@ -493,14 +510,16 @@ static int __kprobes trampoline_probe_handler(struct kprobe *p,
> */
> return 1;
> }
> +NOKPROBE_SYMBOL(trampoline_probe_handler);
>
> -int __kprobes arch_trampoline_kprobe(struct kprobe *p)
> +int arch_trampoline_kprobe(struct kprobe *p)
> {
> if (p->addr == (kprobe_opcode_t *)__kretprobe_trampoline)
> return 1;
>
> return 0;
> }
> +NOKPROBE_SYMBOL(arch_trampoline_kprobe);
>
> static struct kprobe trampoline_p = {
> .addr = (kprobe_opcode_t *)__kretprobe_trampoline,
> diff --git a/arch/mips/mm/fault.c b/arch/mips/mm/fault.c
> index 44f9810..b08bc55 100644
> --- a/arch/mips/mm/fault.c
> +++ b/arch/mips/mm/fault.c
> @@ -35,7 +35,7 @@ int show_unhandled_signals = 1;
> * and the problem, and then passes it off to one of the appropriate
> * routines.
> */
> -static void __kprobes __do_page_fault(struct pt_regs *regs, unsigned long write,
> +static void __do_page_fault(struct pt_regs *regs, unsigned long write,
> unsigned long address)
> {
> struct vm_area_struct * vma = NULL;
> @@ -322,8 +322,9 @@ static void __kprobes __do_page_fault(struct pt_regs *regs, unsigned long write,
> }
> #endif
> }
> +NOKPROBE_SYMBOL(__do_page_fault);
>
> -asmlinkage void __kprobes do_page_fault(struct pt_regs *regs,
> +asmlinkage void do_page_fault(struct pt_regs *regs,
> unsigned long write, unsigned long address)
> {
> enum ctx_state prev_state;
> @@ -332,3 +333,4 @@ asmlinkage void __kprobes do_page_fault(struct pt_regs *regs,
> __do_page_fault(regs, write, address);
> exception_exit(prev_state);
> }
> +NOKPROBE_SYMBOL(do_page_fault);
> --
> 2.1.0
>
--
Masami Hiramatsu <mhiramat@kernel.org>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] selftests/ftrace: Save kprobe_events to test log
2022-05-05 12:19 ` [PATCH 1/2] selftests/ftrace: Save kprobe_events to test log Tiezhu Yang
@ 2022-05-06 1:46 ` Masami Hiramatsu
0 siblings, 0 replies; 6+ messages in thread
From: Masami Hiramatsu @ 2022-05-06 1:46 UTC (permalink / raw)
To: Tiezhu Yang
Cc: Thomas Bogendoerfer, Steven Rostedt, Shuah Khan, Xuefeng Li,
linux-mips, linux-kselftest, linux-kernel
On Thu, 5 May 2022 20:19:07 +0800
Tiezhu Yang <yangtiezhu@loongson.cn> wrote:
> It may lead to kernel panic when execute the following testcase on mips:
>
> # ./ftracetest test.d/kprobe/multiple_kprobes.tc
>
> A preliminary analysis shows that the issue is related with
>
> echo 1 > events/kprobes/enable
>
> after add the 256 probe points.
>
> In order to find the root cause, I want to verify which probe point has
> problem, so it is necessary to save kprobe_events to test log.
>
> With this patch, we can get the 256 probe points in the test log through
> the following command:
>
> # ./ftracetest test.d/kprobe/multiple_kprobes.tc -vvv -k
This looks good to me. :)
Acked-by: Masami Hiramatsu <mhiramat@kernel.org>
Thank you,
>
> Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
> ---
> tools/testing/selftests/ftrace/test.d/kprobe/multiple_kprobes.tc | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/tools/testing/selftests/ftrace/test.d/kprobe/multiple_kprobes.tc b/tools/testing/selftests/ftrace/test.d/kprobe/multiple_kprobes.tc
> index 312d237..be754f5 100644
> --- a/tools/testing/selftests/ftrace/test.d/kprobe/multiple_kprobes.tc
> +++ b/tools/testing/selftests/ftrace/test.d/kprobe/multiple_kprobes.tc
> @@ -25,6 +25,8 @@ if [ $L -ne 256 ]; then
> exit_fail
> fi
>
> +cat kprobe_events >> $testlog
> +
> echo 1 > events/kprobes/enable
> echo 0 > events/kprobes/enable
> echo > kprobe_events
> --
> 2.1.0
>
--
Masami Hiramatsu <mhiramat@kernel.org>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] MIPS: Use NOKPROBE_SYMBOL() instead of __kprobes annotation
2022-05-06 1:45 ` Masami Hiramatsu
@ 2022-05-06 3:24 ` Tiezhu Yang
0 siblings, 0 replies; 6+ messages in thread
From: Tiezhu Yang @ 2022-05-06 3:24 UTC (permalink / raw)
To: Masami Hiramatsu
Cc: Thomas Bogendoerfer, Steven Rostedt, Shuah Khan, Xuefeng Li,
linux-mips, linux-kselftest, linux-kernel
On 05/06/2022 09:45 AM, Masami Hiramatsu wrote:
> On Thu, 5 May 2022 20:19:08 +0800
> Tiezhu Yang <yangtiezhu@loongson.cn> wrote:
>
>> If define CONFIG_KPROBES, __kprobes annotation forces the whole function
>> into the ".kprobes.text" section, NOKPROBE_SYMBOL() just stores the given
>> function address in the "_kprobe_blacklist" section which is introduced
>> to maintain kprobes blacklist.
>>
>> Modify the related code to use NOKPROBE_SYMBOL() to protect functions from
>> kprobes instead of __kprobes annotation under arch/mips.
>
> So you added some non '__kprobes' annotated functions to NOKPROBE_SYMBOL()
> in this patch. Those caused the kernel panic, right?
Oh, no, this patch is not related with the kernel panic, what it does
is just to use NOKPROBE_SYMBOL() instead of __kprobes annotation, and
also add some non '__kprobes' annotated functions in kprobes.c to
NOKPROBE_SYMBOL() because they can not be probed too.
> If so, please add such
> comment on this description too. Or, split this into 2 patches, one fixes
> the kernel panic by adding those functions to NOKPROBE_SYMBOL() and the
> other is replacing __kprobes with NOKPROBE_SYMBOL().
>
> Also, could you also find the commit which introduces the kernel panic?
> It is worth to backport such fix to stable trees.
I did some work to find some other functions lead to kernel panic,
mark them as notrace function can avoid the problems, but I am not
quite sure the root cause, so I do not submit the changes now.
Thanks,
Tiezhu
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-05-06 3:24 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-05 12:19 [PATCH 0/2] Modify some code about kprobe Tiezhu Yang
2022-05-05 12:19 ` [PATCH 1/2] selftests/ftrace: Save kprobe_events to test log Tiezhu Yang
2022-05-06 1:46 ` Masami Hiramatsu
2022-05-05 12:19 ` [PATCH 2/2] MIPS: Use NOKPROBE_SYMBOL() instead of __kprobes annotation Tiezhu Yang
2022-05-06 1:45 ` Masami Hiramatsu
2022-05-06 3:24 ` Tiezhu Yang
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.