[v12,4/5] LoongArch: Mark some assembler symbols as non-kprobe-able

Message ID 1674007261-9198-5-git-send-email-yangtiezhu@loongson.cn
State New
Headers
Series Add kprobe and kretprobe support for LoongArch |

Commit Message

Tiezhu Yang Jan. 18, 2023, 2:01 a.m. UTC
  Some assembler symbols are not kprobe safe, such as handle_syscall
(used as syscall exception handler), *memcpy* (may cause recursive
exceptions), they can not be instrumented, just blacklist them for
kprobing.

Here is a related problem and discussion:
Link: https://lore.kernel.org/lkml/20230114143859.7ccc45c1c5d9ce302113ab0a@kernel.org/

Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
---
 arch/loongarch/include/asm/asm.h | 10 ++++++++++
 arch/loongarch/kernel/entry.S    |  1 +
 arch/loongarch/lib/memcpy.S      |  3 +++
 3 files changed, 14 insertions(+)
  

Comments

Huacai Chen Jan. 18, 2023, 4:14 a.m. UTC | #1
If memcpy should be blacklisted, then what about memset and memmove?

Huacai

On Wed, Jan 18, 2023 at 10:01 AM Tiezhu Yang <yangtiezhu@loongson.cn> wrote:
>
> Some assembler symbols are not kprobe safe, such as handle_syscall
> (used as syscall exception handler), *memcpy* (may cause recursive
> exceptions), they can not be instrumented, just blacklist them for
> kprobing.
>
> Here is a related problem and discussion:
> Link: https://lore.kernel.org/lkml/20230114143859.7ccc45c1c5d9ce302113ab0a@kernel.org/
>
> Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
> ---
>  arch/loongarch/include/asm/asm.h | 10 ++++++++++
>  arch/loongarch/kernel/entry.S    |  1 +
>  arch/loongarch/lib/memcpy.S      |  3 +++
>  3 files changed, 14 insertions(+)
>
> diff --git a/arch/loongarch/include/asm/asm.h b/arch/loongarch/include/asm/asm.h
> index 40eea6a..f591b32 100644
> --- a/arch/loongarch/include/asm/asm.h
> +++ b/arch/loongarch/include/asm/asm.h
> @@ -188,4 +188,14 @@
>  #define PTRLOG         3
>  #endif
>
> +/* Annotate a function as being unsuitable for kprobes. */
> +#ifdef CONFIG_KPROBES
> +#define _ASM_NOKPROBE(name)                            \
> +       .pushsection "_kprobe_blacklist", "aw";         \
> +       .quad   name;                                   \
> +       .popsection
> +#else
> +#define _ASM_NOKPROBE(name)
> +#endif
> +
>  #endif /* __ASM_ASM_H */
> diff --git a/arch/loongarch/kernel/entry.S b/arch/loongarch/kernel/entry.S
> index d53b631..55e23b1 100644
> --- a/arch/loongarch/kernel/entry.S
> +++ b/arch/loongarch/kernel/entry.S
> @@ -67,6 +67,7 @@ SYM_FUNC_START(handle_syscall)
>
>         RESTORE_ALL_AND_RET
>  SYM_FUNC_END(handle_syscall)
> +_ASM_NOKPROBE(handle_syscall)
>
>  SYM_CODE_START(ret_from_fork)
>         bl      schedule_tail           # a0 = struct task_struct *prev
> diff --git a/arch/loongarch/lib/memcpy.S b/arch/loongarch/lib/memcpy.S
> index 7c07d59..3b7e1de 100644
> --- a/arch/loongarch/lib/memcpy.S
> +++ b/arch/loongarch/lib/memcpy.S
> @@ -17,6 +17,7 @@ SYM_FUNC_START(memcpy)
>         ALTERNATIVE     "b __memcpy_generic", \
>                         "b __memcpy_fast", CPU_FEATURE_UAL
>  SYM_FUNC_END(memcpy)
> +_ASM_NOKPROBE(memcpy)
>
>  EXPORT_SYMBOL(memcpy)
>
> @@ -41,6 +42,7 @@ SYM_FUNC_START(__memcpy_generic)
>  2:     move    a0, a3
>         jr      ra
>  SYM_FUNC_END(__memcpy_generic)
> +_ASM_NOKPROBE(__memcpy_generic)
>
>  /*
>   * void *__memcpy_fast(void *dst, const void *src, size_t n)
> @@ -93,3 +95,4 @@ SYM_FUNC_START(__memcpy_fast)
>  3:     move    a0, a3
>         jr      ra
>  SYM_FUNC_END(__memcpy_fast)
> +_ASM_NOKPROBE(__memcpy_fast)
> --
> 2.1.0
>
  
Tiezhu Yang Jan. 18, 2023, 4:23 a.m. UTC | #2
On 01/18/2023 12:14 PM, Huacai Chen wrote:
> If memcpy should be blacklisted, then what about memset and memmove?

According to the test results, there are no problems to probe
memset and memmove, so no need to blacklist them for now,
blacklist memcpy is because it may cause recursive exceptions,
there is a detailed discussion in the following link:

https://lore.kernel.org/lkml/20230114143859.7ccc45c1c5d9ce302113ab0a@kernel.org/

Thanks,
Tiezhu

>
> Huacai
>
> On Wed, Jan 18, 2023 at 10:01 AM Tiezhu Yang <yangtiezhu@loongson.cn> wrote:
>>
>> Some assembler symbols are not kprobe safe, such as handle_syscall
>> (used as syscall exception handler), *memcpy* (may cause recursive
>> exceptions), they can not be instrumented, just blacklist them for
>> kprobing.
>>
>> Here is a related problem and discussion:
>> Link: https://lore.kernel.org/lkml/20230114143859.7ccc45c1c5d9ce302113ab0a@kernel.org/
>>
>> Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
>> ---
>>  arch/loongarch/include/asm/asm.h | 10 ++++++++++
>>  arch/loongarch/kernel/entry.S    |  1 +
>>  arch/loongarch/lib/memcpy.S      |  3 +++
>>  3 files changed, 14 insertions(+)
>>
>> diff --git a/arch/loongarch/include/asm/asm.h b/arch/loongarch/include/asm/asm.h
>> index 40eea6a..f591b32 100644
>> --- a/arch/loongarch/include/asm/asm.h
>> +++ b/arch/loongarch/include/asm/asm.h
>> @@ -188,4 +188,14 @@
>>  #define PTRLOG         3
>>  #endif
>>
>> +/* Annotate a function as being unsuitable for kprobes. */
>> +#ifdef CONFIG_KPROBES
>> +#define _ASM_NOKPROBE(name)                            \
>> +       .pushsection "_kprobe_blacklist", "aw";         \
>> +       .quad   name;                                   \
>> +       .popsection
>> +#else
>> +#define _ASM_NOKPROBE(name)
>> +#endif
>> +
>>  #endif /* __ASM_ASM_H */
>> diff --git a/arch/loongarch/kernel/entry.S b/arch/loongarch/kernel/entry.S
>> index d53b631..55e23b1 100644
>> --- a/arch/loongarch/kernel/entry.S
>> +++ b/arch/loongarch/kernel/entry.S
>> @@ -67,6 +67,7 @@ SYM_FUNC_START(handle_syscall)
>>
>>         RESTORE_ALL_AND_RET
>>  SYM_FUNC_END(handle_syscall)
>> +_ASM_NOKPROBE(handle_syscall)
>>
>>  SYM_CODE_START(ret_from_fork)
>>         bl      schedule_tail           # a0 = struct task_struct *prev
>> diff --git a/arch/loongarch/lib/memcpy.S b/arch/loongarch/lib/memcpy.S
>> index 7c07d59..3b7e1de 100644
>> --- a/arch/loongarch/lib/memcpy.S
>> +++ b/arch/loongarch/lib/memcpy.S
>> @@ -17,6 +17,7 @@ SYM_FUNC_START(memcpy)
>>         ALTERNATIVE     "b __memcpy_generic", \
>>                         "b __memcpy_fast", CPU_FEATURE_UAL
>>  SYM_FUNC_END(memcpy)
>> +_ASM_NOKPROBE(memcpy)
>>
>>  EXPORT_SYMBOL(memcpy)
>>
>> @@ -41,6 +42,7 @@ SYM_FUNC_START(__memcpy_generic)
>>  2:     move    a0, a3
>>         jr      ra
>>  SYM_FUNC_END(__memcpy_generic)
>> +_ASM_NOKPROBE(__memcpy_generic)
>>
>>  /*
>>   * void *__memcpy_fast(void *dst, const void *src, size_t n)
>> @@ -93,3 +95,4 @@ SYM_FUNC_START(__memcpy_fast)
>>  3:     move    a0, a3
>>         jr      ra
>>  SYM_FUNC_END(__memcpy_fast)
>> +_ASM_NOKPROBE(__memcpy_fast)
>> --
>> 2.1.0
>>
  
Jinyang He Jan. 18, 2023, 6:05 a.m. UTC | #3
On 2023-01-18 12:23, Tiezhu Yang wrote:
>
>
> On 01/18/2023 12:14 PM, Huacai Chen wrote:
>> If memcpy should be blacklisted, then what about memset and memmove?
>
> According to the test results, there are no problems to probe
> memset and memmove, so no need to blacklist them for now,
> blacklist memcpy is because it may cause recursive exceptions,
> there is a detailed discussion in the following link:
>
> https://lore.kernel.org/lkml/20230114143859.7ccc45c1c5d9ce302113ab0a@kernel.org/ 
>

Hi, Tiezhu,

I cannot reproduce the results when kprobe memcpy. Could you please give
some details. Emm, I just replace "kernel_clone" with "memcpy" in
kprobe_example.c.

And for your call trace,

  handler_pre()
    pr_info()
      printk()
       _printk()
         vprintk()
           vprintk_store()
             memcpy()

I think when we should skip this time kprobe which triggered in
handler_{pre, post}. That means this time kprobe will not call
handler_{pre, post} agian, and not cause recursion. I remember
your codes had done this skip action. So, that's so strange if
recursion in handler_{pre, post}.


Thanks,

Jinyang


>
> Thanks,
> Tiezhu
>
>>
>> Huacai
>>
>> On Wed, Jan 18, 2023 at 10:01 AM Tiezhu Yang <yangtiezhu@loongson.cn> 
>> wrote:
>>>
>>> Some assembler symbols are not kprobe safe, such as handle_syscall
>>> (used as syscall exception handler), *memcpy* (may cause recursive
>>> exceptions), they can not be instrumented, just blacklist them for
>>> kprobing.
>>>
>>> Here is a related problem and discussion:
>>> Link: 
>>> https://lore.kernel.org/lkml/20230114143859.7ccc45c1c5d9ce302113ab0a@kernel.org/
>>>
>>> Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
>>> ---
>>>  arch/loongarch/include/asm/asm.h | 10 ++++++++++
>>>  arch/loongarch/kernel/entry.S    |  1 +
>>>  arch/loongarch/lib/memcpy.S      |  3 +++
>>>  3 files changed, 14 insertions(+)
>>>
>>> diff --git a/arch/loongarch/include/asm/asm.h 
>>> b/arch/loongarch/include/asm/asm.h
>>> index 40eea6a..f591b32 100644
>>> --- a/arch/loongarch/include/asm/asm.h
>>> +++ b/arch/loongarch/include/asm/asm.h
>>> @@ -188,4 +188,14 @@
>>>  #define PTRLOG         3
>>>  #endif
>>>
>>> +/* Annotate a function as being unsuitable for kprobes. */
>>> +#ifdef CONFIG_KPROBES
>>> +#define _ASM_NOKPROBE(name)                            \
>>> +       .pushsection "_kprobe_blacklist", "aw";         \
>>> +       .quad   name;                                   \
>>> +       .popsection
>>> +#else
>>> +#define _ASM_NOKPROBE(name)
>>> +#endif
>>> +
>>>  #endif /* __ASM_ASM_H */
>>> diff --git a/arch/loongarch/kernel/entry.S 
>>> b/arch/loongarch/kernel/entry.S
>>> index d53b631..55e23b1 100644
>>> --- a/arch/loongarch/kernel/entry.S
>>> +++ b/arch/loongarch/kernel/entry.S
>>> @@ -67,6 +67,7 @@ SYM_FUNC_START(handle_syscall)
>>>
>>>         RESTORE_ALL_AND_RET
>>>  SYM_FUNC_END(handle_syscall)
>>> +_ASM_NOKPROBE(handle_syscall)
>>>
>>>  SYM_CODE_START(ret_from_fork)
>>>         bl      schedule_tail           # a0 = struct task_struct *prev
>>> diff --git a/arch/loongarch/lib/memcpy.S b/arch/loongarch/lib/memcpy.S
>>> index 7c07d59..3b7e1de 100644
>>> --- a/arch/loongarch/lib/memcpy.S
>>> +++ b/arch/loongarch/lib/memcpy.S
>>> @@ -17,6 +17,7 @@ SYM_FUNC_START(memcpy)
>>>         ALTERNATIVE     "b __memcpy_generic", \
>>>                         "b __memcpy_fast", CPU_FEATURE_UAL
>>>  SYM_FUNC_END(memcpy)
>>> +_ASM_NOKPROBE(memcpy)
>>>
>>>  EXPORT_SYMBOL(memcpy)
>>>
>>> @@ -41,6 +42,7 @@ SYM_FUNC_START(__memcpy_generic)
>>>  2:     move    a0, a3
>>>         jr      ra
>>>  SYM_FUNC_END(__memcpy_generic)
>>> +_ASM_NOKPROBE(__memcpy_generic)
>>>
>>>  /*
>>>   * void *__memcpy_fast(void *dst, const void *src, size_t n)
>>> @@ -93,3 +95,4 @@ SYM_FUNC_START(__memcpy_fast)
>>>  3:     move    a0, a3
>>>         jr      ra
>>>  SYM_FUNC_END(__memcpy_fast)
>>> +_ASM_NOKPROBE(__memcpy_fast)
>>> -- 
>>> 2.1.0
>>>
>
  
Tiezhu Yang Jan. 18, 2023, 6:24 a.m. UTC | #4
On 01/18/2023 02:05 PM, Jinyang He wrote:
>
> On 2023-01-18 12:23, Tiezhu Yang wrote:
>>
>>
>> On 01/18/2023 12:14 PM, Huacai Chen wrote:
>>> If memcpy should be blacklisted, then what about memset and memmove?
>>
>> According to the test results, there are no problems to probe
>> memset and memmove, so no need to blacklist them for now,
>> blacklist memcpy is because it may cause recursive exceptions,
>> there is a detailed discussion in the following link:
>>
>> https://lore.kernel.org/lkml/20230114143859.7ccc45c1c5d9ce302113ab0a@kernel.org/
>>
>
> Hi, Tiezhu,
>
> I cannot reproduce the results when kprobe memcpy. Could you please give
> some details. Emm, I just replace "kernel_clone" with "memcpy" in
> kprobe_example.c.

Please remove the related "_ASM_NOKPROBE(memcpy)" code in
arch/loongarch/lib/memcpy.S, and then compile and update kernel,
execute the following cmd after reboot, I can reproduce the hang
problem easily (it will take a few minutes).

modprobe kprobe_example symbol="memcpy"

>
> And for your call trace,
>
>  handler_pre()
>    pr_info()
>      printk()
>       _printk()
>         vprintk()
>           vprintk_store()
>             memcpy()
>
> I think when we should skip this time kprobe which triggered in
> handler_{pre, post}. That means this time kprobe will not call
> handler_{pre, post} agian, and not cause recursion. I remember
> your codes had done this skip action. So, that's so strange if
> recursion in handler_{pre, post}.
>
>
> Thanks,
>
> Jinyang
>
>
>>
>> Thanks,
>> Tiezhu
>>
>>>
>>> Huacai
>>>
>>> On Wed, Jan 18, 2023 at 10:01 AM Tiezhu Yang <yangtiezhu@loongson.cn>
>>> wrote:
>>>>
>>>> Some assembler symbols are not kprobe safe, such as handle_syscall
>>>> (used as syscall exception handler), *memcpy* (may cause recursive
>>>> exceptions), they can not be instrumented, just blacklist them for
>>>> kprobing.
>>>>
>>>> Here is a related problem and discussion:
>>>> Link:
>>>> https://lore.kernel.org/lkml/20230114143859.7ccc45c1c5d9ce302113ab0a@kernel.org/
>>>>
>>>>
>>>> Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
>>>> ---
>>>>  arch/loongarch/include/asm/asm.h | 10 ++++++++++
>>>>  arch/loongarch/kernel/entry.S    |  1 +
>>>>  arch/loongarch/lib/memcpy.S      |  3 +++
>>>>  3 files changed, 14 insertions(+)
>>>>
>>>> diff --git a/arch/loongarch/include/asm/asm.h
>>>> b/arch/loongarch/include/asm/asm.h
>>>> index 40eea6a..f591b32 100644
>>>> --- a/arch/loongarch/include/asm/asm.h
>>>> +++ b/arch/loongarch/include/asm/asm.h
>>>> @@ -188,4 +188,14 @@
>>>>  #define PTRLOG         3
>>>>  #endif
>>>>
>>>> +/* Annotate a function as being unsuitable for kprobes. */
>>>> +#ifdef CONFIG_KPROBES
>>>> +#define _ASM_NOKPROBE(name)                            \
>>>> +       .pushsection "_kprobe_blacklist", "aw";         \
>>>> +       .quad   name;                                   \
>>>> +       .popsection
>>>> +#else
>>>> +#define _ASM_NOKPROBE(name)
>>>> +#endif
>>>> +
>>>>  #endif /* __ASM_ASM_H */
>>>> diff --git a/arch/loongarch/kernel/entry.S
>>>> b/arch/loongarch/kernel/entry.S
>>>> index d53b631..55e23b1 100644
>>>> --- a/arch/loongarch/kernel/entry.S
>>>> +++ b/arch/loongarch/kernel/entry.S
>>>> @@ -67,6 +67,7 @@ SYM_FUNC_START(handle_syscall)
>>>>
>>>>         RESTORE_ALL_AND_RET
>>>>  SYM_FUNC_END(handle_syscall)
>>>> +_ASM_NOKPROBE(handle_syscall)
>>>>
>>>>  SYM_CODE_START(ret_from_fork)
>>>>         bl      schedule_tail           # a0 = struct task_struct *prev
>>>> diff --git a/arch/loongarch/lib/memcpy.S b/arch/loongarch/lib/memcpy.S
>>>> index 7c07d59..3b7e1de 100644
>>>> --- a/arch/loongarch/lib/memcpy.S
>>>> +++ b/arch/loongarch/lib/memcpy.S
>>>> @@ -17,6 +17,7 @@ SYM_FUNC_START(memcpy)
>>>>         ALTERNATIVE     "b __memcpy_generic", \
>>>>                         "b __memcpy_fast", CPU_FEATURE_UAL
>>>>  SYM_FUNC_END(memcpy)
>>>> +_ASM_NOKPROBE(memcpy)
>>>>
>>>>  EXPORT_SYMBOL(memcpy)
>>>>
>>>> @@ -41,6 +42,7 @@ SYM_FUNC_START(__memcpy_generic)
>>>>  2:     move    a0, a3
>>>>         jr      ra
>>>>  SYM_FUNC_END(__memcpy_generic)
>>>> +_ASM_NOKPROBE(__memcpy_generic)
>>>>
>>>>  /*
>>>>   * void *__memcpy_fast(void *dst, const void *src, size_t n)
>>>> @@ -93,3 +95,4 @@ SYM_FUNC_START(__memcpy_fast)
>>>>  3:     move    a0, a3
>>>>         jr      ra
>>>>  SYM_FUNC_END(__memcpy_fast)
>>>> +_ASM_NOKPROBE(__memcpy_fast)
>>>> --
>>>> 2.1.0
>>>>
>>
  
Huacai Chen Jan. 18, 2023, 7:17 a.m. UTC | #5
On Wed, Jan 18, 2023 at 2:24 PM Tiezhu Yang <yangtiezhu@loongson.cn> wrote:
>
>
>
> On 01/18/2023 02:05 PM, Jinyang He wrote:
> >
> > On 2023-01-18 12:23, Tiezhu Yang wrote:
> >>
> >>
> >> On 01/18/2023 12:14 PM, Huacai Chen wrote:
> >>> If memcpy should be blacklisted, then what about memset and memmove?
> >>
> >> According to the test results, there are no problems to probe
> >> memset and memmove, so no need to blacklist them for now,
> >> blacklist memcpy is because it may cause recursive exceptions,
> >> there is a detailed discussion in the following link:
> >>
> >> https://lore.kernel.org/lkml/20230114143859.7ccc45c1c5d9ce302113ab0a@kernel.org/
> >>
> >
> > Hi, Tiezhu,
> >
> > I cannot reproduce the results when kprobe memcpy. Could you please give
> > some details. Emm, I just replace "kernel_clone" with "memcpy" in
> > kprobe_example.c.
>
> Please remove the related "_ASM_NOKPROBE(memcpy)" code in
> arch/loongarch/lib/memcpy.S, and then compile and update kernel,
> execute the following cmd after reboot, I can reproduce the hang
> problem easily (it will take a few minutes).
>
> modprobe kprobe_example symbol="memcpy"
Then, why is handle_syscall different from other exception handlers?

Huacai
>
> >
> > And for your call trace,
> >
> >  handler_pre()
> >    pr_info()
> >      printk()
> >       _printk()
> >         vprintk()
> >           vprintk_store()
> >             memcpy()
> >
> > I think when we should skip this time kprobe which triggered in
> > handler_{pre, post}. That means this time kprobe will not call
> > handler_{pre, post} agian, and not cause recursion. I remember
> > your codes had done this skip action. So, that's so strange if
> > recursion in handler_{pre, post}.
> >
> >
> > Thanks,
> >
> > Jinyang
> >
> >
> >>
> >> Thanks,
> >> Tiezhu
> >>
> >>>
> >>> Huacai
> >>>
> >>> On Wed, Jan 18, 2023 at 10:01 AM Tiezhu Yang <yangtiezhu@loongson.cn>
> >>> wrote:
> >>>>
> >>>> Some assembler symbols are not kprobe safe, such as handle_syscall
> >>>> (used as syscall exception handler), *memcpy* (may cause recursive
> >>>> exceptions), they can not be instrumented, just blacklist them for
> >>>> kprobing.
> >>>>
> >>>> Here is a related problem and discussion:
> >>>> Link:
> >>>> https://lore.kernel.org/lkml/20230114143859.7ccc45c1c5d9ce302113ab0a@kernel.org/
> >>>>
> >>>>
> >>>> Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
> >>>> ---
> >>>>  arch/loongarch/include/asm/asm.h | 10 ++++++++++
> >>>>  arch/loongarch/kernel/entry.S    |  1 +
> >>>>  arch/loongarch/lib/memcpy.S      |  3 +++
> >>>>  3 files changed, 14 insertions(+)
> >>>>
> >>>> diff --git a/arch/loongarch/include/asm/asm.h
> >>>> b/arch/loongarch/include/asm/asm.h
> >>>> index 40eea6a..f591b32 100644
> >>>> --- a/arch/loongarch/include/asm/asm.h
> >>>> +++ b/arch/loongarch/include/asm/asm.h
> >>>> @@ -188,4 +188,14 @@
> >>>>  #define PTRLOG         3
> >>>>  #endif
> >>>>
> >>>> +/* Annotate a function as being unsuitable for kprobes. */
> >>>> +#ifdef CONFIG_KPROBES
> >>>> +#define _ASM_NOKPROBE(name)                            \
> >>>> +       .pushsection "_kprobe_blacklist", "aw";         \
> >>>> +       .quad   name;                                   \
> >>>> +       .popsection
> >>>> +#else
> >>>> +#define _ASM_NOKPROBE(name)
> >>>> +#endif
> >>>> +
> >>>>  #endif /* __ASM_ASM_H */
> >>>> diff --git a/arch/loongarch/kernel/entry.S
> >>>> b/arch/loongarch/kernel/entry.S
> >>>> index d53b631..55e23b1 100644
> >>>> --- a/arch/loongarch/kernel/entry.S
> >>>> +++ b/arch/loongarch/kernel/entry.S
> >>>> @@ -67,6 +67,7 @@ SYM_FUNC_START(handle_syscall)
> >>>>
> >>>>         RESTORE_ALL_AND_RET
> >>>>  SYM_FUNC_END(handle_syscall)
> >>>> +_ASM_NOKPROBE(handle_syscall)
> >>>>
> >>>>  SYM_CODE_START(ret_from_fork)
> >>>>         bl      schedule_tail           # a0 = struct task_struct *prev
> >>>> diff --git a/arch/loongarch/lib/memcpy.S b/arch/loongarch/lib/memcpy.S
> >>>> index 7c07d59..3b7e1de 100644
> >>>> --- a/arch/loongarch/lib/memcpy.S
> >>>> +++ b/arch/loongarch/lib/memcpy.S
> >>>> @@ -17,6 +17,7 @@ SYM_FUNC_START(memcpy)
> >>>>         ALTERNATIVE     "b __memcpy_generic", \
> >>>>                         "b __memcpy_fast", CPU_FEATURE_UAL
> >>>>  SYM_FUNC_END(memcpy)
> >>>> +_ASM_NOKPROBE(memcpy)
> >>>>
> >>>>  EXPORT_SYMBOL(memcpy)
> >>>>
> >>>> @@ -41,6 +42,7 @@ SYM_FUNC_START(__memcpy_generic)
> >>>>  2:     move    a0, a3
> >>>>         jr      ra
> >>>>  SYM_FUNC_END(__memcpy_generic)
> >>>> +_ASM_NOKPROBE(__memcpy_generic)
> >>>>
> >>>>  /*
> >>>>   * void *__memcpy_fast(void *dst, const void *src, size_t n)
> >>>> @@ -93,3 +95,4 @@ SYM_FUNC_START(__memcpy_fast)
> >>>>  3:     move    a0, a3
> >>>>         jr      ra
> >>>>  SYM_FUNC_END(__memcpy_fast)
> >>>> +_ASM_NOKPROBE(__memcpy_fast)
> >>>> --
> >>>> 2.1.0
> >>>>
> >>
>
>
  
Jinyang He Jan. 18, 2023, 7:20 a.m. UTC | #6
On 2023-01-18 14:24, Tiezhu Yang wrote:
>
>
> On 01/18/2023 02:05 PM, Jinyang He wrote:
>>
>> On 2023-01-18 12:23, Tiezhu Yang wrote:
>>>
>>>
>>> On 01/18/2023 12:14 PM, Huacai Chen wrote:
>>>> If memcpy should be blacklisted, then what about memset and memmove?
>>>
>>> According to the test results, there are no problems to probe
>>> memset and memmove, so no need to blacklist them for now,
>>> blacklist memcpy is because it may cause recursive exceptions,
>>> there is a detailed discussion in the following link:
>>>
>>> https://lore.kernel.org/lkml/20230114143859.7ccc45c1c5d9ce302113ab0a@kernel.org/ 
>>>
>>>
>>
>> Hi, Tiezhu,
>>
>> I cannot reproduce the results when kprobe memcpy. Could you please give
>> some details. Emm, I just replace "kernel_clone" with "memcpy" in
>> kprobe_example.c.
>
> Please remove the related "_ASM_NOKPROBE(memcpy)" code in
> arch/loongarch/lib/memcpy.S, and then compile and update kernel,
> execute the following cmd after reboot, I can reproduce the hang
> problem easily (it will take a few minutes).
>
> modprobe kprobe_example symbol="memcpy"

Okay, I can reproduce the hang, but sometimes quickly while
sometimes slowly. I do not know why it happend. Can you
explain how recursion happens? I means, can you explain why

no need mark {vprintk_store, vprintk, ... } as it may also cause recursion.


>
>>
>> And for your call trace,
>>
>>  handler_pre()
>>    pr_info()
>>      printk()
>>       _printk()
>>         vprintk()
>>           vprintk_store()
>>             memcpy()
>>
>> I think when we should skip this time kprobe which triggered in
>> handler_{pre, post}. That means this time kprobe will not call
>> handler_{pre, post} agian, and not cause recursion. I remember
>> your codes had done this skip action. So, that's so strange if
>> recursion in handler_{pre, post}.
>>
>>
>> Thanks,
>>
>> Jinyang
>>
>>
>>>
>>> Thanks,
>>> Tiezhu
>>>
>>>>
>>>> Huacai
>>>>
>>>> On Wed, Jan 18, 2023 at 10:01 AM Tiezhu Yang <yangtiezhu@loongson.cn>
>>>> wrote:
>>>>>
>>>>> Some assembler symbols are not kprobe safe, such as handle_syscall
>>>>> (used as syscall exception handler), *memcpy* (may cause recursive
>>>>> exceptions), they can not be instrumented, just blacklist them for
>>>>> kprobing.
>>>>>
>>>>> Here is a related problem and discussion:
>>>>> Link:
>>>>> https://lore.kernel.org/lkml/20230114143859.7ccc45c1c5d9ce302113ab0a@kernel.org/ 
>>>>>
>>>>>
>>>>>
>>>>> Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
>>>>> ---
>>>>>  arch/loongarch/include/asm/asm.h | 10 ++++++++++
>>>>>  arch/loongarch/kernel/entry.S    |  1 +
>>>>>  arch/loongarch/lib/memcpy.S      |  3 +++
>>>>>  3 files changed, 14 insertions(+)
>>>>>
>>>>> diff --git a/arch/loongarch/include/asm/asm.h
>>>>> b/arch/loongarch/include/asm/asm.h
>>>>> index 40eea6a..f591b32 100644
>>>>> --- a/arch/loongarch/include/asm/asm.h
>>>>> +++ b/arch/loongarch/include/asm/asm.h
>>>>> @@ -188,4 +188,14 @@
>>>>>  #define PTRLOG         3
>>>>>  #endif
>>>>>
>>>>> +/* Annotate a function as being unsuitable for kprobes. */
>>>>> +#ifdef CONFIG_KPROBES
>>>>> +#define _ASM_NOKPROBE(name)                            \
>>>>> +       .pushsection "_kprobe_blacklist", "aw";         \
>>>>> +       .quad   name;                                   \
>>>>> +       .popsection
>>>>> +#else
>>>>> +#define _ASM_NOKPROBE(name)
>>>>> +#endif
>>>>> +
>>>>>  #endif /* __ASM_ASM_H */
>>>>> diff --git a/arch/loongarch/kernel/entry.S
>>>>> b/arch/loongarch/kernel/entry.S
>>>>> index d53b631..55e23b1 100644
>>>>> --- a/arch/loongarch/kernel/entry.S
>>>>> +++ b/arch/loongarch/kernel/entry.S
>>>>> @@ -67,6 +67,7 @@ SYM_FUNC_START(handle_syscall)
>>>>>
>>>>>         RESTORE_ALL_AND_RET
>>>>>  SYM_FUNC_END(handle_syscall)
>>>>> +_ASM_NOKPROBE(handle_syscall)
>>>>>
>>>>>  SYM_CODE_START(ret_from_fork)
>>>>>         bl      schedule_tail           # a0 = struct task_struct 
>>>>> *prev
>>>>> diff --git a/arch/loongarch/lib/memcpy.S 
>>>>> b/arch/loongarch/lib/memcpy.S
>>>>> index 7c07d59..3b7e1de 100644
>>>>> --- a/arch/loongarch/lib/memcpy.S
>>>>> +++ b/arch/loongarch/lib/memcpy.S
>>>>> @@ -17,6 +17,7 @@ SYM_FUNC_START(memcpy)
>>>>>         ALTERNATIVE     "b __memcpy_generic", \
>>>>>                         "b __memcpy_fast", CPU_FEATURE_UAL
>>>>>  SYM_FUNC_END(memcpy)
>>>>> +_ASM_NOKPROBE(memcpy)
>>>>>
>>>>>  EXPORT_SYMBOL(memcpy)
>>>>>
>>>>> @@ -41,6 +42,7 @@ SYM_FUNC_START(__memcpy_generic)
>>>>>  2:     move    a0, a3
>>>>>         jr      ra
>>>>>  SYM_FUNC_END(__memcpy_generic)
>>>>> +_ASM_NOKPROBE(__memcpy_generic)
>>>>>
>>>>>  /*
>>>>>   * void *__memcpy_fast(void *dst, const void *src, size_t n)
>>>>> @@ -93,3 +95,4 @@ SYM_FUNC_START(__memcpy_fast)
>>>>>  3:     move    a0, a3
>>>>>         jr      ra
>>>>>  SYM_FUNC_END(__memcpy_fast)
>>>>> +_ASM_NOKPROBE(__memcpy_fast)
>>>>> -- 
>>>>> 2.1.0
>>>>>
>>>
  
Masami Hiramatsu (Google) Jan. 19, 2023, 3:31 p.m. UTC | #7
On Wed, 18 Jan 2023 15:17:00 +0800
Huacai Chen <chenhuacai@kernel.org> wrote:

> On Wed, Jan 18, 2023 at 2:24 PM Tiezhu Yang <yangtiezhu@loongson.cn> wrote:
> >
> >
> >
> > On 01/18/2023 02:05 PM, Jinyang He wrote:
> > >
> > > On 2023-01-18 12:23, Tiezhu Yang wrote:
> > >>
> > >>
> > >> On 01/18/2023 12:14 PM, Huacai Chen wrote:
> > >>> If memcpy should be blacklisted, then what about memset and memmove?
> > >>
> > >> According to the test results, there are no problems to probe
> > >> memset and memmove, so no need to blacklist them for now,
> > >> blacklist memcpy is because it may cause recursive exceptions,
> > >> there is a detailed discussion in the following link:
> > >>
> > >> https://lore.kernel.org/lkml/20230114143859.7ccc45c1c5d9ce302113ab0a@kernel.org/
> > >>
> > >
> > > Hi, Tiezhu,
> > >
> > > I cannot reproduce the results when kprobe memcpy. Could you please give
> > > some details. Emm, I just replace "kernel_clone" with "memcpy" in
> > > kprobe_example.c.
> >
> > Please remove the related "_ASM_NOKPROBE(memcpy)" code in
> > arch/loongarch/lib/memcpy.S, and then compile and update kernel,
> > execute the following cmd after reboot, I can reproduce the hang
> > problem easily (it will take a few minutes).
> >
> > modprobe kprobe_example symbol="memcpy"
> Then, why is handle_syscall different from other exception handlers?

I need to check the loongarch implementation of handle_syscall() but
I guess in that handler the register set is not completely set as
kernel one. In that case, the software breakpoint handler may not
possible to handle it correctly. So it is better to avoid probing such
"border" function by kprobes.

Thank you,

> 
> Huacai
> >
> > >
> > > And for your call trace,
> > >
> > >  handler_pre()
> > >    pr_info()
> > >      printk()
> > >       _printk()
> > >         vprintk()
> > >           vprintk_store()
> > >             memcpy()
> > >
> > > I think when we should skip this time kprobe which triggered in
> > > handler_{pre, post}. That means this time kprobe will not call
> > > handler_{pre, post} agian, and not cause recursion. I remember
> > > your codes had done this skip action. So, that's so strange if
> > > recursion in handler_{pre, post}.
> > >
> > >
> > > Thanks,
> > >
> > > Jinyang
> > >
> > >
> > >>
> > >> Thanks,
> > >> Tiezhu
> > >>
> > >>>
> > >>> Huacai
> > >>>
> > >>> On Wed, Jan 18, 2023 at 10:01 AM Tiezhu Yang <yangtiezhu@loongson.cn>
> > >>> wrote:
> > >>>>
> > >>>> Some assembler symbols are not kprobe safe, such as handle_syscall
> > >>>> (used as syscall exception handler), *memcpy* (may cause recursive
> > >>>> exceptions), they can not be instrumented, just blacklist them for
> > >>>> kprobing.
> > >>>>
> > >>>> Here is a related problem and discussion:
> > >>>> Link:
> > >>>> https://lore.kernel.org/lkml/20230114143859.7ccc45c1c5d9ce302113ab0a@kernel.org/
> > >>>>
> > >>>>
> > >>>> Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
> > >>>> ---
> > >>>>  arch/loongarch/include/asm/asm.h | 10 ++++++++++
> > >>>>  arch/loongarch/kernel/entry.S    |  1 +
> > >>>>  arch/loongarch/lib/memcpy.S      |  3 +++
> > >>>>  3 files changed, 14 insertions(+)
> > >>>>
> > >>>> diff --git a/arch/loongarch/include/asm/asm.h
> > >>>> b/arch/loongarch/include/asm/asm.h
> > >>>> index 40eea6a..f591b32 100644
> > >>>> --- a/arch/loongarch/include/asm/asm.h
> > >>>> +++ b/arch/loongarch/include/asm/asm.h
> > >>>> @@ -188,4 +188,14 @@
> > >>>>  #define PTRLOG         3
> > >>>>  #endif
> > >>>>
> > >>>> +/* Annotate a function as being unsuitable for kprobes. */
> > >>>> +#ifdef CONFIG_KPROBES
> > >>>> +#define _ASM_NOKPROBE(name)                            \
> > >>>> +       .pushsection "_kprobe_blacklist", "aw";         \
> > >>>> +       .quad   name;                                   \
> > >>>> +       .popsection
> > >>>> +#else
> > >>>> +#define _ASM_NOKPROBE(name)
> > >>>> +#endif
> > >>>> +
> > >>>>  #endif /* __ASM_ASM_H */
> > >>>> diff --git a/arch/loongarch/kernel/entry.S
> > >>>> b/arch/loongarch/kernel/entry.S
> > >>>> index d53b631..55e23b1 100644
> > >>>> --- a/arch/loongarch/kernel/entry.S
> > >>>> +++ b/arch/loongarch/kernel/entry.S
> > >>>> @@ -67,6 +67,7 @@ SYM_FUNC_START(handle_syscall)
> > >>>>
> > >>>>         RESTORE_ALL_AND_RET
> > >>>>  SYM_FUNC_END(handle_syscall)
> > >>>> +_ASM_NOKPROBE(handle_syscall)
> > >>>>
> > >>>>  SYM_CODE_START(ret_from_fork)
> > >>>>         bl      schedule_tail           # a0 = struct task_struct *prev
> > >>>> diff --git a/arch/loongarch/lib/memcpy.S b/arch/loongarch/lib/memcpy.S
> > >>>> index 7c07d59..3b7e1de 100644
> > >>>> --- a/arch/loongarch/lib/memcpy.S
> > >>>> +++ b/arch/loongarch/lib/memcpy.S
> > >>>> @@ -17,6 +17,7 @@ SYM_FUNC_START(memcpy)
> > >>>>         ALTERNATIVE     "b __memcpy_generic", \
> > >>>>                         "b __memcpy_fast", CPU_FEATURE_UAL
> > >>>>  SYM_FUNC_END(memcpy)
> > >>>> +_ASM_NOKPROBE(memcpy)
> > >>>>
> > >>>>  EXPORT_SYMBOL(memcpy)
> > >>>>
> > >>>> @@ -41,6 +42,7 @@ SYM_FUNC_START(__memcpy_generic)
> > >>>>  2:     move    a0, a3
> > >>>>         jr      ra
> > >>>>  SYM_FUNC_END(__memcpy_generic)
> > >>>> +_ASM_NOKPROBE(__memcpy_generic)
> > >>>>
> > >>>>  /*
> > >>>>   * void *__memcpy_fast(void *dst, const void *src, size_t n)
> > >>>> @@ -93,3 +95,4 @@ SYM_FUNC_START(__memcpy_fast)
> > >>>>  3:     move    a0, a3
> > >>>>         jr      ra
> > >>>>  SYM_FUNC_END(__memcpy_fast)
> > >>>> +_ASM_NOKPROBE(__memcpy_fast)
> > >>>> --
> > >>>> 2.1.0
> > >>>>
> > >>
> >
> >
  
Huacai Chen Jan. 20, 2023, 1:23 p.m. UTC | #8
On Thu, Jan 19, 2023 at 11:32 PM Masami Hiramatsu <mhiramat@kernel.org> wrote:
>
> On Wed, 18 Jan 2023 15:17:00 +0800
> Huacai Chen <chenhuacai@kernel.org> wrote:
>
> > On Wed, Jan 18, 2023 at 2:24 PM Tiezhu Yang <yangtiezhu@loongson.cn> wrote:
> > >
> > >
> > >
> > > On 01/18/2023 02:05 PM, Jinyang He wrote:
> > > >
> > > > On 2023-01-18 12:23, Tiezhu Yang wrote:
> > > >>
> > > >>
> > > >> On 01/18/2023 12:14 PM, Huacai Chen wrote:
> > > >>> If memcpy should be blacklisted, then what about memset and memmove?
> > > >>
> > > >> According to the test results, there are no problems to probe
> > > >> memset and memmove, so no need to blacklist them for now,
> > > >> blacklist memcpy is because it may cause recursive exceptions,
> > > >> there is a detailed discussion in the following link:
> > > >>
> > > >> https://lore.kernel.org/lkml/20230114143859.7ccc45c1c5d9ce302113ab0a@kernel.org/
> > > >>
> > > >
> > > > Hi, Tiezhu,
> > > >
> > > > I cannot reproduce the results when kprobe memcpy. Could you please give
> > > > some details. Emm, I just replace "kernel_clone" with "memcpy" in
> > > > kprobe_example.c.
> > >
> > > Please remove the related "_ASM_NOKPROBE(memcpy)" code in
> > > arch/loongarch/lib/memcpy.S, and then compile and update kernel,
> > > execute the following cmd after reboot, I can reproduce the hang
> > > problem easily (it will take a few minutes).
> > >
> > > modprobe kprobe_example symbol="memcpy"
> > Then, why is handle_syscall different from other exception handlers?
>
> I need to check the loongarch implementation of handle_syscall() but
> I guess in that handler the register set is not completely set as
> kernel one. In that case, the software breakpoint handler may not
> possible to handle it correctly. So it is better to avoid probing such
> "border" function by kprobes.
Seems reasonable, handle_syscall() indeed doesn't save all registers.
But for memcpy(), I still think memmove() and memset() may have the
same problem.

Huacai
>
> Thank you,
>
> >
> > Huacai
> > >
> > > >
> > > > And for your call trace,
> > > >
> > > >  handler_pre()
> > > >    pr_info()
> > > >      printk()
> > > >       _printk()
> > > >         vprintk()
> > > >           vprintk_store()
> > > >             memcpy()
> > > >
> > > > I think when we should skip this time kprobe which triggered in
> > > > handler_{pre, post}. That means this time kprobe will not call
> > > > handler_{pre, post} agian, and not cause recursion. I remember
> > > > your codes had done this skip action. So, that's so strange if
> > > > recursion in handler_{pre, post}.
> > > >
> > > >
> > > > Thanks,
> > > >
> > > > Jinyang
> > > >
> > > >
> > > >>
> > > >> Thanks,
> > > >> Tiezhu
> > > >>
> > > >>>
> > > >>> Huacai
> > > >>>
> > > >>> On Wed, Jan 18, 2023 at 10:01 AM Tiezhu Yang <yangtiezhu@loongson.cn>
> > > >>> wrote:
> > > >>>>
> > > >>>> Some assembler symbols are not kprobe safe, such as handle_syscall
> > > >>>> (used as syscall exception handler), *memcpy* (may cause recursive
> > > >>>> exceptions), they can not be instrumented, just blacklist them for
> > > >>>> kprobing.
> > > >>>>
> > > >>>> Here is a related problem and discussion:
> > > >>>> Link:
> > > >>>> https://lore.kernel.org/lkml/20230114143859.7ccc45c1c5d9ce302113ab0a@kernel.org/
> > > >>>>
> > > >>>>
> > > >>>> Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
> > > >>>> ---
> > > >>>>  arch/loongarch/include/asm/asm.h | 10 ++++++++++
> > > >>>>  arch/loongarch/kernel/entry.S    |  1 +
> > > >>>>  arch/loongarch/lib/memcpy.S      |  3 +++
> > > >>>>  3 files changed, 14 insertions(+)
> > > >>>>
> > > >>>> diff --git a/arch/loongarch/include/asm/asm.h
> > > >>>> b/arch/loongarch/include/asm/asm.h
> > > >>>> index 40eea6a..f591b32 100644
> > > >>>> --- a/arch/loongarch/include/asm/asm.h
> > > >>>> +++ b/arch/loongarch/include/asm/asm.h
> > > >>>> @@ -188,4 +188,14 @@
> > > >>>>  #define PTRLOG         3
> > > >>>>  #endif
> > > >>>>
> > > >>>> +/* Annotate a function as being unsuitable for kprobes. */
> > > >>>> +#ifdef CONFIG_KPROBES
> > > >>>> +#define _ASM_NOKPROBE(name)                            \
> > > >>>> +       .pushsection "_kprobe_blacklist", "aw";         \
> > > >>>> +       .quad   name;                                   \
> > > >>>> +       .popsection
> > > >>>> +#else
> > > >>>> +#define _ASM_NOKPROBE(name)
> > > >>>> +#endif
> > > >>>> +
> > > >>>>  #endif /* __ASM_ASM_H */
> > > >>>> diff --git a/arch/loongarch/kernel/entry.S
> > > >>>> b/arch/loongarch/kernel/entry.S
> > > >>>> index d53b631..55e23b1 100644
> > > >>>> --- a/arch/loongarch/kernel/entry.S
> > > >>>> +++ b/arch/loongarch/kernel/entry.S
> > > >>>> @@ -67,6 +67,7 @@ SYM_FUNC_START(handle_syscall)
> > > >>>>
> > > >>>>         RESTORE_ALL_AND_RET
> > > >>>>  SYM_FUNC_END(handle_syscall)
> > > >>>> +_ASM_NOKPROBE(handle_syscall)
> > > >>>>
> > > >>>>  SYM_CODE_START(ret_from_fork)
> > > >>>>         bl      schedule_tail           # a0 = struct task_struct *prev
> > > >>>> diff --git a/arch/loongarch/lib/memcpy.S b/arch/loongarch/lib/memcpy.S
> > > >>>> index 7c07d59..3b7e1de 100644
> > > >>>> --- a/arch/loongarch/lib/memcpy.S
> > > >>>> +++ b/arch/loongarch/lib/memcpy.S
> > > >>>> @@ -17,6 +17,7 @@ SYM_FUNC_START(memcpy)
> > > >>>>         ALTERNATIVE     "b __memcpy_generic", \
> > > >>>>                         "b __memcpy_fast", CPU_FEATURE_UAL
> > > >>>>  SYM_FUNC_END(memcpy)
> > > >>>> +_ASM_NOKPROBE(memcpy)
> > > >>>>
> > > >>>>  EXPORT_SYMBOL(memcpy)
> > > >>>>
> > > >>>> @@ -41,6 +42,7 @@ SYM_FUNC_START(__memcpy_generic)
> > > >>>>  2:     move    a0, a3
> > > >>>>         jr      ra
> > > >>>>  SYM_FUNC_END(__memcpy_generic)
> > > >>>> +_ASM_NOKPROBE(__memcpy_generic)
> > > >>>>
> > > >>>>  /*
> > > >>>>   * void *__memcpy_fast(void *dst, const void *src, size_t n)
> > > >>>> @@ -93,3 +95,4 @@ SYM_FUNC_START(__memcpy_fast)
> > > >>>>  3:     move    a0, a3
> > > >>>>         jr      ra
> > > >>>>  SYM_FUNC_END(__memcpy_fast)
> > > >>>> +_ASM_NOKPROBE(__memcpy_fast)
> > > >>>> --
> > > >>>> 2.1.0
> > > >>>>
> > > >>
> > >
> > >
>
>
> --
> Masami Hiramatsu (Google) <mhiramat@kernel.org>
  
Masami Hiramatsu (Google) Feb. 1, 2023, 12:55 p.m. UTC | #9
On Fri, 20 Jan 2023 21:23:18 +0800
Huacai Chen <chenhuacai@kernel.org> wrote:

> On Thu, Jan 19, 2023 at 11:32 PM Masami Hiramatsu <mhiramat@kernel.org> wrote:
> >
> > On Wed, 18 Jan 2023 15:17:00 +0800
> > Huacai Chen <chenhuacai@kernel.org> wrote:
> >
> > > On Wed, Jan 18, 2023 at 2:24 PM Tiezhu Yang <yangtiezhu@loongson.cn> wrote:
> > > >
> > > >
> > > >
> > > > On 01/18/2023 02:05 PM, Jinyang He wrote:
> > > > >
> > > > > On 2023-01-18 12:23, Tiezhu Yang wrote:
> > > > >>
> > > > >>
> > > > >> On 01/18/2023 12:14 PM, Huacai Chen wrote:
> > > > >>> If memcpy should be blacklisted, then what about memset and memmove?
> > > > >>
> > > > >> According to the test results, there are no problems to probe
> > > > >> memset and memmove, so no need to blacklist them for now,
> > > > >> blacklist memcpy is because it may cause recursive exceptions,
> > > > >> there is a detailed discussion in the following link:
> > > > >>
> > > > >> https://lore.kernel.org/lkml/20230114143859.7ccc45c1c5d9ce302113ab0a@kernel.org/
> > > > >>
> > > > >
> > > > > Hi, Tiezhu,
> > > > >
> > > > > I cannot reproduce the results when kprobe memcpy. Could you please give
> > > > > some details. Emm, I just replace "kernel_clone" with "memcpy" in
> > > > > kprobe_example.c.
> > > >
> > > > Please remove the related "_ASM_NOKPROBE(memcpy)" code in
> > > > arch/loongarch/lib/memcpy.S, and then compile and update kernel,
> > > > execute the following cmd after reboot, I can reproduce the hang
> > > > problem easily (it will take a few minutes).
> > > >
> > > > modprobe kprobe_example symbol="memcpy"
> > > Then, why is handle_syscall different from other exception handlers?
> >
> > I need to check the loongarch implementation of handle_syscall() but
> > I guess in that handler the register set is not completely set as
> > kernel one. In that case, the software breakpoint handler may not
> > possible to handle it correctly. So it is better to avoid probing such
> > "border" function by kprobes.
> Seems reasonable, handle_syscall() indeed doesn't save all registers.
> But for memcpy(), I still think memmove() and memset() may have the
> same problem.

I agree with that. Those fundamental functions can be used from some
debug macros in the software breakpoint code in the future (or already?)
Maybe you would better to check it with enabling some more (intrusive)
debug features.

Thank you,

> 
> Huacai
> >
> > Thank you,
> >
> > >
> > > Huacai
> > > >
> > > > >
> > > > > And for your call trace,
> > > > >
> > > > >  handler_pre()
> > > > >    pr_info()
> > > > >      printk()
> > > > >       _printk()
> > > > >         vprintk()
> > > > >           vprintk_store()
> > > > >             memcpy()
> > > > >
> > > > > I think when we should skip this time kprobe which triggered in
> > > > > handler_{pre, post}. That means this time kprobe will not call
> > > > > handler_{pre, post} agian, and not cause recursion. I remember
> > > > > your codes had done this skip action. So, that's so strange if
> > > > > recursion in handler_{pre, post}.
> > > > >
> > > > >
> > > > > Thanks,
> > > > >
> > > > > Jinyang
> > > > >
> > > > >
> > > > >>
> > > > >> Thanks,
> > > > >> Tiezhu
> > > > >>
> > > > >>>
> > > > >>> Huacai
> > > > >>>
> > > > >>> On Wed, Jan 18, 2023 at 10:01 AM Tiezhu Yang <yangtiezhu@loongson.cn>
> > > > >>> wrote:
> > > > >>>>
> > > > >>>> Some assembler symbols are not kprobe safe, such as handle_syscall
> > > > >>>> (used as syscall exception handler), *memcpy* (may cause recursive
> > > > >>>> exceptions), they can not be instrumented, just blacklist them for
> > > > >>>> kprobing.
> > > > >>>>
> > > > >>>> Here is a related problem and discussion:
> > > > >>>> Link:
> > > > >>>> https://lore.kernel.org/lkml/20230114143859.7ccc45c1c5d9ce302113ab0a@kernel.org/
> > > > >>>>
> > > > >>>>
> > > > >>>> Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
> > > > >>>> ---
> > > > >>>>  arch/loongarch/include/asm/asm.h | 10 ++++++++++
> > > > >>>>  arch/loongarch/kernel/entry.S    |  1 +
> > > > >>>>  arch/loongarch/lib/memcpy.S      |  3 +++
> > > > >>>>  3 files changed, 14 insertions(+)
> > > > >>>>
> > > > >>>> diff --git a/arch/loongarch/include/asm/asm.h
> > > > >>>> b/arch/loongarch/include/asm/asm.h
> > > > >>>> index 40eea6a..f591b32 100644
> > > > >>>> --- a/arch/loongarch/include/asm/asm.h
> > > > >>>> +++ b/arch/loongarch/include/asm/asm.h
> > > > >>>> @@ -188,4 +188,14 @@
> > > > >>>>  #define PTRLOG         3
> > > > >>>>  #endif
> > > > >>>>
> > > > >>>> +/* Annotate a function as being unsuitable for kprobes. */
> > > > >>>> +#ifdef CONFIG_KPROBES
> > > > >>>> +#define _ASM_NOKPROBE(name)                            \
> > > > >>>> +       .pushsection "_kprobe_blacklist", "aw";         \
> > > > >>>> +       .quad   name;                                   \
> > > > >>>> +       .popsection
> > > > >>>> +#else
> > > > >>>> +#define _ASM_NOKPROBE(name)
> > > > >>>> +#endif
> > > > >>>> +
> > > > >>>>  #endif /* __ASM_ASM_H */
> > > > >>>> diff --git a/arch/loongarch/kernel/entry.S
> > > > >>>> b/arch/loongarch/kernel/entry.S
> > > > >>>> index d53b631..55e23b1 100644
> > > > >>>> --- a/arch/loongarch/kernel/entry.S
> > > > >>>> +++ b/arch/loongarch/kernel/entry.S
> > > > >>>> @@ -67,6 +67,7 @@ SYM_FUNC_START(handle_syscall)
> > > > >>>>
> > > > >>>>         RESTORE_ALL_AND_RET
> > > > >>>>  SYM_FUNC_END(handle_syscall)
> > > > >>>> +_ASM_NOKPROBE(handle_syscall)
> > > > >>>>
> > > > >>>>  SYM_CODE_START(ret_from_fork)
> > > > >>>>         bl      schedule_tail           # a0 = struct task_struct *prev
> > > > >>>> diff --git a/arch/loongarch/lib/memcpy.S b/arch/loongarch/lib/memcpy.S
> > > > >>>> index 7c07d59..3b7e1de 100644
> > > > >>>> --- a/arch/loongarch/lib/memcpy.S
> > > > >>>> +++ b/arch/loongarch/lib/memcpy.S
> > > > >>>> @@ -17,6 +17,7 @@ SYM_FUNC_START(memcpy)
> > > > >>>>         ALTERNATIVE     "b __memcpy_generic", \
> > > > >>>>                         "b __memcpy_fast", CPU_FEATURE_UAL
> > > > >>>>  SYM_FUNC_END(memcpy)
> > > > >>>> +_ASM_NOKPROBE(memcpy)
> > > > >>>>
> > > > >>>>  EXPORT_SYMBOL(memcpy)
> > > > >>>>
> > > > >>>> @@ -41,6 +42,7 @@ SYM_FUNC_START(__memcpy_generic)
> > > > >>>>  2:     move    a0, a3
> > > > >>>>         jr      ra
> > > > >>>>  SYM_FUNC_END(__memcpy_generic)
> > > > >>>> +_ASM_NOKPROBE(__memcpy_generic)
> > > > >>>>
> > > > >>>>  /*
> > > > >>>>   * void *__memcpy_fast(void *dst, const void *src, size_t n)
> > > > >>>> @@ -93,3 +95,4 @@ SYM_FUNC_START(__memcpy_fast)
> > > > >>>>  3:     move    a0, a3
> > > > >>>>         jr      ra
> > > > >>>>  SYM_FUNC_END(__memcpy_fast)
> > > > >>>> +_ASM_NOKPROBE(__memcpy_fast)
> > > > >>>> --
> > > > >>>> 2.1.0
> > > > >>>>
> > > > >>
> > > >
> > > >
> >
> >
> > --
> > Masami Hiramatsu (Google) <mhiramat@kernel.org>
  

Patch

diff --git a/arch/loongarch/include/asm/asm.h b/arch/loongarch/include/asm/asm.h
index 40eea6a..f591b32 100644
--- a/arch/loongarch/include/asm/asm.h
+++ b/arch/loongarch/include/asm/asm.h
@@ -188,4 +188,14 @@ 
 #define PTRLOG		3
 #endif
 
+/* Annotate a function as being unsuitable for kprobes. */
+#ifdef CONFIG_KPROBES
+#define _ASM_NOKPROBE(name)				\
+	.pushsection "_kprobe_blacklist", "aw";		\
+	.quad	name;					\
+	.popsection
+#else
+#define _ASM_NOKPROBE(name)
+#endif
+
 #endif /* __ASM_ASM_H */
diff --git a/arch/loongarch/kernel/entry.S b/arch/loongarch/kernel/entry.S
index d53b631..55e23b1 100644
--- a/arch/loongarch/kernel/entry.S
+++ b/arch/loongarch/kernel/entry.S
@@ -67,6 +67,7 @@  SYM_FUNC_START(handle_syscall)
 
 	RESTORE_ALL_AND_RET
 SYM_FUNC_END(handle_syscall)
+_ASM_NOKPROBE(handle_syscall)
 
 SYM_CODE_START(ret_from_fork)
 	bl	schedule_tail		# a0 = struct task_struct *prev
diff --git a/arch/loongarch/lib/memcpy.S b/arch/loongarch/lib/memcpy.S
index 7c07d59..3b7e1de 100644
--- a/arch/loongarch/lib/memcpy.S
+++ b/arch/loongarch/lib/memcpy.S
@@ -17,6 +17,7 @@  SYM_FUNC_START(memcpy)
 	ALTERNATIVE	"b __memcpy_generic", \
 			"b __memcpy_fast", CPU_FEATURE_UAL
 SYM_FUNC_END(memcpy)
+_ASM_NOKPROBE(memcpy)
 
 EXPORT_SYMBOL(memcpy)
 
@@ -41,6 +42,7 @@  SYM_FUNC_START(__memcpy_generic)
 2:	move	a0, a3
 	jr	ra
 SYM_FUNC_END(__memcpy_generic)
+_ASM_NOKPROBE(__memcpy_generic)
 
 /*
  * void *__memcpy_fast(void *dst, const void *src, size_t n)
@@ -93,3 +95,4 @@  SYM_FUNC_START(__memcpy_fast)
 3:	move	a0, a3
 	jr	ra
 SYM_FUNC_END(__memcpy_fast)
+_ASM_NOKPROBE(__memcpy_fast)