2023-01-11 12:03:42

by Tiezhu Yang

[permalink] [raw]
Subject: kernel hangs when kprobe memcpy

Hi all,

(1) I have the following test environment, kernel hangs when kprobe memcpy:

system: x86_64 fedora 36
kernel version: Linux 5.7 (compile and update)
test case: modprobe kprobe_example symbol="memcpy" (CONFIG_SAMPLE_KPROBES=m)

In order to fix build errors, it needs to unset CONFIG_NFP and do the
following changes:
commit 52a9dab6d892 ("libsubcmd: Fix use-after-free for realloc(..., 0)")
commit de979c83574a ("x86/entry: Build thunk_$(BITS) only if
CONFIG_PREEMPTION=y")

(2) Using the latest upstream mainline kernel, no hang problem due to the
commit e3a9e681adb7 ("x86/entry: Fixup bad_iret vs noinstr") to prohibit
probing memcpy which is put into the .noinstr.text section.

# modprobe kprobe_example symbol="memcpy"
modprobe: ERROR: could not insert 'kprobe_example': Invalid argument

In my opinion, according to the commit message, the above commit is not
intended to fix the memcpy hang problem, the problem was fixed by accident.

(3) If make handler_pre() and handler_post() as empty functions in the 5.7
kernel code, the above hang problem does not exist.

diff --git a/samples/kprobes/kprobe_example.c
b/samples/kprobes/kprobe_example.c
index fd346f58ddba..c194171d8a46 100644
--- a/samples/kprobes/kprobe_example.c
+++ b/samples/kprobes/kprobe_example.c
@@ -28,8 +28,6 @@ static struct kprobe kp = {
static int __kprobes handler_pre(struct kprobe *p, struct pt_regs *regs)
{
#ifdef CONFIG_X86
- pr_info("<%s> p->addr = 0x%p, ip = %lx, flags = 0x%lx\n",
- p->symbol_name, p->addr, regs->ip, regs->flags);
#endif
#ifdef CONFIG_PPC
pr_info("<%s> p->addr = 0x%p, nip = 0x%lx, msr = 0x%lx\n",
@@ -65,8 +63,6 @@ static void __kprobes handler_post(struct kprobe *p,
struct pt_regs *regs,
unsigned long flags)
{
#ifdef CONFIG_X86
- pr_info("<%s> p->addr = 0x%p, flags = 0x%lx\n",
- p->symbol_name, p->addr, regs->flags);
#endif
#ifdef CONFIG_PPC
pr_info("<%s> p->addr = 0x%p, msr = 0x%lx\n",

I want to know what is the real reason of the hang problem when kprobe
memcpy,
I guess it may be kprobe recursion, what do you think? Thank you.

By the way, kprobe memset has no problem whether or not handler_pre() and
handler_post() are empty functions.

Thanks,
Tiezhu


2023-01-12 15:07:15

by Tiezhu Yang

[permalink] [raw]
Subject: Re: kernel hangs when kprobe memcpy



On 01/11/2023 07:38 PM, Tiezhu Yang wrote:
> Hi all,
>
> (1) I have the following test environment, kernel hangs when kprobe memcpy:
>
> system: x86_64 fedora 36
> kernel version: Linux 5.7 (compile and update)
> test case: modprobe kprobe_example symbol="memcpy"
> (CONFIG_SAMPLE_KPROBES=m)
>
> In order to fix build errors, it needs to unset CONFIG_NFP and do the
> following changes:
> commit 52a9dab6d892 ("libsubcmd: Fix use-after-free for realloc(..., 0)")
> commit de979c83574a ("x86/entry: Build thunk_$(BITS) only if
> CONFIG_PREEMPTION=y")
>
> (2) Using the latest upstream mainline kernel, no hang problem due to the
> commit e3a9e681adb7 ("x86/entry: Fixup bad_iret vs noinstr") to prohibit
> probing memcpy which is put into the .noinstr.text section.
>
> # modprobe kprobe_example symbol="memcpy"
> modprobe: ERROR: could not insert 'kprobe_example': Invalid argument
>
> In my opinion, according to the commit message, the above commit is not
> intended to fix the memcpy hang problem, the problem was fixed by accident.
>
> (3) If make handler_pre() and handler_post() as empty functions in the 5.7
> kernel code, the above hang problem does not exist.
>
> diff --git a/samples/kprobes/kprobe_example.c
> b/samples/kprobes/kprobe_example.c
> index fd346f58ddba..c194171d8a46 100644
> --- a/samples/kprobes/kprobe_example.c
> +++ b/samples/kprobes/kprobe_example.c
> @@ -28,8 +28,6 @@ static struct kprobe kp = {
> static int __kprobes handler_pre(struct kprobe *p, struct pt_regs *regs)
> {
> #ifdef CONFIG_X86
> - pr_info("<%s> p->addr = 0x%p, ip = %lx, flags = 0x%lx\n",
> - p->symbol_name, p->addr, regs->ip, regs->flags);
> #endif
> #ifdef CONFIG_PPC
> pr_info("<%s> p->addr = 0x%p, nip = 0x%lx, msr = 0x%lx\n",
> @@ -65,8 +63,6 @@ static void __kprobes handler_post(struct kprobe *p,
> struct pt_regs *regs,
> unsigned long flags)
> {
> #ifdef CONFIG_X86
> - pr_info("<%s> p->addr = 0x%p, flags = 0x%lx\n",
> - p->symbol_name, p->addr, regs->flags);
> #endif
> #ifdef CONFIG_PPC
> pr_info("<%s> p->addr = 0x%p, msr = 0x%lx\n",
>
> I want to know what is the real reason of the hang problem when kprobe
> memcpy,
> I guess it may be kprobe recursion, what do you think? Thank you.
>
> By the way, kprobe memset has no problem whether or not handler_pre() and
> handler_post() are empty functions.
>
> Thanks,
> Tiezhu

I find out the following call trace:

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

I think it may cause recursive exceptions, so we should
mark memcpy as non-kprobe-able, right?

Thanks,
Tiezhu

2023-01-12 15:22:56

by Masami Hiramatsu

[permalink] [raw]
Subject: Re: kernel hangs when kprobe memcpy

Hi Tiezhu,

On Thu, 12 Jan 2023 21:32:51 +0800
Tiezhu Yang <[email protected]> wrote:

>
>
> On 01/11/2023 07:38 PM, Tiezhu Yang wrote:
> > Hi all,
> >
> > (1) I have the following test environment, kernel hangs when kprobe memcpy:
> >
> > system: x86_64 fedora 36
> > kernel version: Linux 5.7 (compile and update)
> > test case: modprobe kprobe_example symbol="memcpy"
> > (CONFIG_SAMPLE_KPROBES=m)
> >
> > In order to fix build errors, it needs to unset CONFIG_NFP and do the
> > following changes:
> > commit 52a9dab6d892 ("libsubcmd: Fix use-after-free for realloc(..., 0)")
> > commit de979c83574a ("x86/entry: Build thunk_$(BITS) only if
> > CONFIG_PREEMPTION=y")
> >
> > (2) Using the latest upstream mainline kernel, no hang problem due to the
> > commit e3a9e681adb7 ("x86/entry: Fixup bad_iret vs noinstr") to prohibit
> > probing memcpy which is put into the .noinstr.text section.
> >
> > # modprobe kprobe_example symbol="memcpy"
> > modprobe: ERROR: could not insert 'kprobe_example': Invalid argument
> >
> > In my opinion, according to the commit message, the above commit is not
> > intended to fix the memcpy hang problem, the problem was fixed by accident.
> >
> > (3) If make handler_pre() and handler_post() as empty functions in the 5.7
> > kernel code, the above hang problem does not exist.


> >
> > diff --git a/samples/kprobes/kprobe_example.c
> > b/samples/kprobes/kprobe_example.c
> > index fd346f58ddba..c194171d8a46 100644
> > --- a/samples/kprobes/kprobe_example.c
> > +++ b/samples/kprobes/kprobe_example.c
> > @@ -28,8 +28,6 @@ static struct kprobe kp = {
> > static int __kprobes handler_pre(struct kprobe *p, struct pt_regs *regs)
> > {
> > #ifdef CONFIG_X86
> > - pr_info("<%s> p->addr = 0x%p, ip = %lx, flags = 0x%lx\n",
> > - p->symbol_name, p->addr, regs->ip, regs->flags);
> > #endif
> > #ifdef CONFIG_PPC
> > pr_info("<%s> p->addr = 0x%p, nip = 0x%lx, msr = 0x%lx\n",
> > @@ -65,8 +63,6 @@ static void __kprobes handler_post(struct kprobe *p,
> > struct pt_regs *regs,
> > unsigned long flags)
> > {
> > #ifdef CONFIG_X86
> > - pr_info("<%s> p->addr = 0x%p, flags = 0x%lx\n",
> > - p->symbol_name, p->addr, regs->flags);
> > #endif
> > #ifdef CONFIG_PPC
> > pr_info("<%s> p->addr = 0x%p, msr = 0x%lx\n",
> >
> > I want to know what is the real reason of the hang problem when kprobe
> > memcpy,
> > I guess it may be kprobe recursion, what do you think? Thank you.
> >
> > By the way, kprobe memset has no problem whether or not handler_pre() and
> > handler_post() are empty functions.
> >
> > Thanks,
> > Tiezhu
>
> I find out the following call trace:
>
> handler_pre()
> pr_info()
> printk()
> _printk()
> vprintk()
> vprintk_store()
> memcpy()
>
> I think it may cause recursive exceptions, so we should
> mark memcpy as non-kprobe-able, right?

Yes, and the .noinstr.text (noinstr function attribute) is including
non-kprobe-able (nokprobe function attribute). I a function is nokprobe
and notrace, it should be noinstr. "NOKPROBE_SYMBOL" is used for the
symbol which is called in the kprobe processing path (e.g. x86 int3
handler etc.).

BTW, that the bug you reported is interesting. Even if another kprobe
is called inside kprobe pre/post handler, it must be skipped.
If you can share your kconfig, I can try to reproduce it.

Thank you,

--
Masami Hiramatsu (Google) <[email protected]>

2023-01-13 07:20:07

by Tiezhu Yang

[permalink] [raw]
Subject: Re: kernel hangs when kprobe memcpy



On 01/12/2023 10:36 PM, Masami Hiramatsu (Google) wrote:
> Hi Tiezhu,
>
> On Thu, 12 Jan 2023 21:32:51 +0800
> Tiezhu Yang <[email protected]> wrote:
>
>>
>>
>> On 01/11/2023 07:38 PM, Tiezhu Yang wrote:
>>> Hi all,
>>>
>>> (1) I have the following test environment, kernel hangs when kprobe memcpy:
>>>
>>> system: x86_64 fedora 36
>>> kernel version: Linux 5.7 (compile and update)
>>> test case: modprobe kprobe_example symbol="memcpy"
>>> (CONFIG_SAMPLE_KPROBES=m)
>>>
>>> In order to fix build errors, it needs to unset CONFIG_NFP and do the
>>> following changes:
>>> commit 52a9dab6d892 ("libsubcmd: Fix use-after-free for realloc(..., 0)")
>>> commit de979c83574a ("x86/entry: Build thunk_$(BITS) only if
>>> CONFIG_PREEMPTION=y")
>>>
>>> (2) Using the latest upstream mainline kernel, no hang problem due to the
>>> commit e3a9e681adb7 ("x86/entry: Fixup bad_iret vs noinstr") to prohibit
>>> probing memcpy which is put into the .noinstr.text section.
>>>
>>> # modprobe kprobe_example symbol="memcpy"
>>> modprobe: ERROR: could not insert 'kprobe_example': Invalid argument
>>>
>>> In my opinion, according to the commit message, the above commit is not
>>> intended to fix the memcpy hang problem, the problem was fixed by accident.
>>>
>>> (3) If make handler_pre() and handler_post() as empty functions in the 5.7
>>> kernel code, the above hang problem does not exist.
>
>
>>>
>>> diff --git a/samples/kprobes/kprobe_example.c
>>> b/samples/kprobes/kprobe_example.c
>>> index fd346f58ddba..c194171d8a46 100644
>>> --- a/samples/kprobes/kprobe_example.c
>>> +++ b/samples/kprobes/kprobe_example.c
>>> @@ -28,8 +28,6 @@ static struct kprobe kp = {
>>> static int __kprobes handler_pre(struct kprobe *p, struct pt_regs *regs)
>>> {
>>> #ifdef CONFIG_X86
>>> - pr_info("<%s> p->addr = 0x%p, ip = %lx, flags = 0x%lx\n",
>>> - p->symbol_name, p->addr, regs->ip, regs->flags);
>>> #endif
>>> #ifdef CONFIG_PPC
>>> pr_info("<%s> p->addr = 0x%p, nip = 0x%lx, msr = 0x%lx\n",
>>> @@ -65,8 +63,6 @@ static void __kprobes handler_post(struct kprobe *p,
>>> struct pt_regs *regs,
>>> unsigned long flags)
>>> {
>>> #ifdef CONFIG_X86
>>> - pr_info("<%s> p->addr = 0x%p, flags = 0x%lx\n",
>>> - p->symbol_name, p->addr, regs->flags);
>>> #endif
>>> #ifdef CONFIG_PPC
>>> pr_info("<%s> p->addr = 0x%p, msr = 0x%lx\n",
>>>
>>> I want to know what is the real reason of the hang problem when kprobe
>>> memcpy,
>>> I guess it may be kprobe recursion, what do you think? Thank you.
>>>
>>> By the way, kprobe memset has no problem whether or not handler_pre() and
>>> handler_post() are empty functions.
>>>
>>> Thanks,
>>> Tiezhu
>>
>> I find out the following call trace:
>>
>> handler_pre()
>> pr_info()
>> printk()
>> _printk()
>> vprintk()
>> vprintk_store()
>> memcpy()
>>
>> I think it may cause recursive exceptions, so we should
>> mark memcpy as non-kprobe-able, right?
>
> Yes, and the .noinstr.text (noinstr function attribute) is including
> non-kprobe-able (nokprobe function attribute). I a function is nokprobe
> and notrace, it should be noinstr. "NOKPROBE_SYMBOL" is used for the
> symbol which is called in the kprobe processing path (e.g. x86 int3
> handler etc.).
>
> BTW, that the bug you reported is interesting. Even if another kprobe
> is called inside kprobe pre/post handler, it must be skipped.
> If you can share your kconfig, I can try to reproduce it.
>
> Thank you,
>

Hi Masami,

Thank you very much for your reply.

Please use the attached config and diff file, here are the steps
to reproduce kernel hangs when kprobe memcpy on x86_64 fedora 36:

(1) kernel 5.7
$ wget --no-check-certificate
https://git.kernel.org/torvalds/t/linux-5.7.tar.gz
$ ls
5.7.config 5.7.diff linux-5.7.tar.gz
$ tar xf linux-5.7.tar.gz
$ cd linux-5.7/
$ patch -p1 < ../5.7.diff
$ cp ../5.7.config .config
$ make -j8
# make modules_install -j8
# make install
# set the default kernel and reboot
# modprobe kprobe_example symbol="memcpy"

(2) kernel 6.2-rc1
$ wget --no-check-certificate
https://git.kernel.org/torvalds/t/linux-6.2-rc1.tar.gz
$ ls
6.2-rc1.config 6.2-rc1.diff linux-6.2-rc1.tar.gz
$ tar xf linux-6.2-rc1.tar.gz
$ cd linux-6.2-rc1/
$ patch -p1 < ../6.2-rc1.diff
$ cp ../6.2-rc1.config .config
$ make -j8
# make modules_install -j8
# make install
# set the default kernel and reboot
# modprobe kprobe_example symbol="memcpy"

By the way, I am developing and testing kprobe on LoongArch, I met the
same hang problems when probe some symbols, such as (1) handle_syscall,
like entry_SYSCALL_64 in arch/x86/entry/entry_64.S, used as syscall
exception handler, (2) memcpy, it may cause recursive exceptions like
x86.

I do the following changes on LoongArch, could you please take a look
whether the code itself and the commit message are proper? Thank you.

LoongArch: Mark some assembler symbols as non-kprobe-able

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.

diff --git a/arch/loongarch/include/asm/asm.h
b/arch/loongarch/include/asm/asm.h
index 40eea6aa469e..c51a0914fc99 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 d53b631c9022..05696bf7b69d 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 7c07d595ee89..b32dd25ce3a4 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)

Thanks,
Tiezhu


Attachments:
5.7.config (225.57 kB)
5.7.diff (2.95 kB)
6.2-rc1.config (260.73 kB)
6.2-rc1.diff (553.00 B)
config-5.17.5-300.fc36.x86_64 (248.14 kB)
Download all attachments

2023-01-14 06:35:40

by Masami Hiramatsu

[permalink] [raw]
Subject: Re: kernel hangs when kprobe memcpy

Hi Tiezhu,

On Fri, 13 Jan 2023 14:26:52 +0800
Tiezhu Yang <[email protected]> wrote:

>
>
> On 01/12/2023 10:36 PM, Masami Hiramatsu (Google) wrote:
> > Hi Tiezhu,
> >
> > On Thu, 12 Jan 2023 21:32:51 +0800
> > Tiezhu Yang <[email protected]> wrote:
> >
> >>
> >>
> >> On 01/11/2023 07:38 PM, Tiezhu Yang wrote:
> >>> Hi all,
> >>>
> >>> (1) I have the following test environment, kernel hangs when kprobe memcpy:
> >>>
> >>> system: x86_64 fedora 36
> >>> kernel version: Linux 5.7 (compile and update)
> >>> test case: modprobe kprobe_example symbol="memcpy"
> >>> (CONFIG_SAMPLE_KPROBES=m)
> >>>
> >>> In order to fix build errors, it needs to unset CONFIG_NFP and do the
> >>> following changes:
> >>> commit 52a9dab6d892 ("libsubcmd: Fix use-after-free for realloc(..., 0)")
> >>> commit de979c83574a ("x86/entry: Build thunk_$(BITS) only if
> >>> CONFIG_PREEMPTION=y")
> >>>
> >>> (2) Using the latest upstream mainline kernel, no hang problem due to the
> >>> commit e3a9e681adb7 ("x86/entry: Fixup bad_iret vs noinstr") to prohibit
> >>> probing memcpy which is put into the .noinstr.text section.
> >>>
> >>> # modprobe kprobe_example symbol="memcpy"
> >>> modprobe: ERROR: could not insert 'kprobe_example': Invalid argument
> >>>
> >>> In my opinion, according to the commit message, the above commit is not
> >>> intended to fix the memcpy hang problem, the problem was fixed by accident.
> >>>
> >>> (3) If make handler_pre() and handler_post() as empty functions in the 5.7
> >>> kernel code, the above hang problem does not exist.
> >
> >
> >>>
> >>> diff --git a/samples/kprobes/kprobe_example.c
> >>> b/samples/kprobes/kprobe_example.c
> >>> index fd346f58ddba..c194171d8a46 100644
> >>> --- a/samples/kprobes/kprobe_example.c
> >>> +++ b/samples/kprobes/kprobe_example.c
> >>> @@ -28,8 +28,6 @@ static struct kprobe kp = {
> >>> static int __kprobes handler_pre(struct kprobe *p, struct pt_regs *regs)
> >>> {
> >>> #ifdef CONFIG_X86
> >>> - pr_info("<%s> p->addr = 0x%p, ip = %lx, flags = 0x%lx\n",
> >>> - p->symbol_name, p->addr, regs->ip, regs->flags);
> >>> #endif
> >>> #ifdef CONFIG_PPC
> >>> pr_info("<%s> p->addr = 0x%p, nip = 0x%lx, msr = 0x%lx\n",
> >>> @@ -65,8 +63,6 @@ static void __kprobes handler_post(struct kprobe *p,
> >>> struct pt_regs *regs,
> >>> unsigned long flags)
> >>> {
> >>> #ifdef CONFIG_X86
> >>> - pr_info("<%s> p->addr = 0x%p, flags = 0x%lx\n",
> >>> - p->symbol_name, p->addr, regs->flags);
> >>> #endif
> >>> #ifdef CONFIG_PPC
> >>> pr_info("<%s> p->addr = 0x%p, msr = 0x%lx\n",
> >>>
> >>> I want to know what is the real reason of the hang problem when kprobe
> >>> memcpy,
> >>> I guess it may be kprobe recursion, what do you think? Thank you.
> >>>
> >>> By the way, kprobe memset has no problem whether or not handler_pre() and
> >>> handler_post() are empty functions.
> >>>
> >>> Thanks,
> >>> Tiezhu
> >>
> >> I find out the following call trace:
> >>
> >> handler_pre()
> >> pr_info()
> >> printk()
> >> _printk()
> >> vprintk()
> >> vprintk_store()
> >> memcpy()
> >>
> >> I think it may cause recursive exceptions, so we should
> >> mark memcpy as non-kprobe-able, right?
> >
> > Yes, and the .noinstr.text (noinstr function attribute) is including
> > non-kprobe-able (nokprobe function attribute). I a function is nokprobe
> > and notrace, it should be noinstr. "NOKPROBE_SYMBOL" is used for the
> > symbol which is called in the kprobe processing path (e.g. x86 int3
> > handler etc.).
> >
> > BTW, that the bug you reported is interesting. Even if another kprobe
> > is called inside kprobe pre/post handler, it must be skipped.
> > If you can share your kconfig, I can try to reproduce it.
> >
> > Thank you,
> >
>
> Hi Masami,
>
> Thank you very much for your reply.
>
> Please use the attached config and diff file, here are the steps
> to reproduce kernel hangs when kprobe memcpy on x86_64 fedora 36:
>
> (1) kernel 5.7
> $ wget --no-check-certificate
> https://git.kernel.org/torvalds/t/linux-5.7.tar.gz
> $ ls
> 5.7.config 5.7.diff linux-5.7.tar.gz
> $ tar xf linux-5.7.tar.gz
> $ cd linux-5.7/
> $ patch -p1 < ../5.7.diff
> $ cp ../5.7.config .config
> $ make -j8
> # make modules_install -j8
> # make install
> # set the default kernel and reboot
> # modprobe kprobe_example symbol="memcpy"
>
> (2) kernel 6.2-rc1
> $ wget --no-check-certificate
> https://git.kernel.org/torvalds/t/linux-6.2-rc1.tar.gz
> $ ls
> 6.2-rc1.config 6.2-rc1.diff linux-6.2-rc1.tar.gz
> $ tar xf linux-6.2-rc1.tar.gz
> $ cd linux-6.2-rc1/
> $ patch -p1 < ../6.2-rc1.diff
> $ cp ../6.2-rc1.config .config
> $ make -j8
> # make modules_install -j8
> # make install
> # set the default kernel and reboot
> # modprobe kprobe_example symbol="memcpy"
>
> By the way, I am developing and testing kprobe on LoongArch, I met the
> same hang problems when probe some symbols, such as (1) handle_syscall,
> like entry_SYSCALL_64 in arch/x86/entry/entry_64.S, used as syscall
> exception handler, (2) memcpy, it may cause recursive exceptions like
> x86.

If you saw that without any change, please report it. At least
memcpy is already marked as noinstr.

>
> I do the following changes on LoongArch, could you please take a look
> whether the code itself and the commit message are proper? Thank you.

Yeah, this direction is good to me.

>
> LoongArch: Mark some assembler symbols as non-kprobe-able
>
> 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.
>
> diff --git a/arch/loongarch/include/asm/asm.h
> b/arch/loongarch/include/asm/asm.h
> index 40eea6aa469e..c51a0914fc99 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) \

As same as other archs, can you rename it _ASM_NOKPROBE()?

Others look good to me.

Thanks!

> + .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 d53b631c9022..05696bf7b69d 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 7c07d595ee89..b32dd25ce3a4 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)
>
> Thanks,
> Tiezhu


--
Masami Hiramatsu (Google) <[email protected]>

2023-01-14 07:11:46

by Tiezhu Yang

[permalink] [raw]
Subject: Re: kernel hangs when kprobe memcpy



On 01/14/2023 01:38 PM, Masami Hiramatsu (Google) wrote:
> Hi Tiezhu,
>
> On Fri, 13 Jan 2023 14:26:52 +0800
> Tiezhu Yang <[email protected]> wrote:
>
>>
>>
>> On 01/12/2023 10:36 PM, Masami Hiramatsu (Google) wrote:
>>> Hi Tiezhu,
>>>
>>> On Thu, 12 Jan 2023 21:32:51 +0800
>>> Tiezhu Yang <[email protected]> wrote:
>>>
>>>>
>>>>
>>>> On 01/11/2023 07:38 PM, Tiezhu Yang wrote:
>>>>> Hi all,
>>>>>
>>>>> (1) I have the following test environment, kernel hangs when kprobe memcpy:
>>>>>
>>>>> system: x86_64 fedora 36
>>>>> kernel version: Linux 5.7 (compile and update)
>>>>> test case: modprobe kprobe_example symbol="memcpy"
>>>>> (CONFIG_SAMPLE_KPROBES=m)
>>>>>
>>>>> In order to fix build errors, it needs to unset CONFIG_NFP and do the
>>>>> following changes:
>>>>> commit 52a9dab6d892 ("libsubcmd: Fix use-after-free for realloc(..., 0)")
>>>>> commit de979c83574a ("x86/entry: Build thunk_$(BITS) only if
>>>>> CONFIG_PREEMPTION=y")
>>>>>
>>>>> (2) Using the latest upstream mainline kernel, no hang problem due to the
>>>>> commit e3a9e681adb7 ("x86/entry: Fixup bad_iret vs noinstr") to prohibit
>>>>> probing memcpy which is put into the .noinstr.text section.
>>>>>
>>>>> # modprobe kprobe_example symbol="memcpy"
>>>>> modprobe: ERROR: could not insert 'kprobe_example': Invalid argument
>>>>>
>>>>> In my opinion, according to the commit message, the above commit is not
>>>>> intended to fix the memcpy hang problem, the problem was fixed by accident.
>>>>>
>>>>> (3) If make handler_pre() and handler_post() as empty functions in the 5.7
>>>>> kernel code, the above hang problem does not exist.
>>>
>>>
>>>>>
>>>>> diff --git a/samples/kprobes/kprobe_example.c
>>>>> b/samples/kprobes/kprobe_example.c
>>>>> index fd346f58ddba..c194171d8a46 100644
>>>>> --- a/samples/kprobes/kprobe_example.c
>>>>> +++ b/samples/kprobes/kprobe_example.c
>>>>> @@ -28,8 +28,6 @@ static struct kprobe kp = {
>>>>> static int __kprobes handler_pre(struct kprobe *p, struct pt_regs *regs)
>>>>> {
>>>>> #ifdef CONFIG_X86
>>>>> - pr_info("<%s> p->addr = 0x%p, ip = %lx, flags = 0x%lx\n",
>>>>> - p->symbol_name, p->addr, regs->ip, regs->flags);
>>>>> #endif
>>>>> #ifdef CONFIG_PPC
>>>>> pr_info("<%s> p->addr = 0x%p, nip = 0x%lx, msr = 0x%lx\n",
>>>>> @@ -65,8 +63,6 @@ static void __kprobes handler_post(struct kprobe *p,
>>>>> struct pt_regs *regs,
>>>>> unsigned long flags)
>>>>> {
>>>>> #ifdef CONFIG_X86
>>>>> - pr_info("<%s> p->addr = 0x%p, flags = 0x%lx\n",
>>>>> - p->symbol_name, p->addr, regs->flags);
>>>>> #endif
>>>>> #ifdef CONFIG_PPC
>>>>> pr_info("<%s> p->addr = 0x%p, msr = 0x%lx\n",
>>>>>
>>>>> I want to know what is the real reason of the hang problem when kprobe
>>>>> memcpy,
>>>>> I guess it may be kprobe recursion, what do you think? Thank you.
>>>>>
>>>>> By the way, kprobe memset has no problem whether or not handler_pre() and
>>>>> handler_post() are empty functions.
>>>>>
>>>>> Thanks,
>>>>> Tiezhu
>>>>
>>>> I find out the following call trace:
>>>>
>>>> handler_pre()
>>>> pr_info()
>>>> printk()
>>>> _printk()
>>>> vprintk()
>>>> vprintk_store()
>>>> memcpy()
>>>>
>>>> I think it may cause recursive exceptions, so we should
>>>> mark memcpy as non-kprobe-able, right?
>>>
>>> Yes, and the .noinstr.text (noinstr function attribute) is including
>>> non-kprobe-able (nokprobe function attribute). I a function is nokprobe
>>> and notrace, it should be noinstr. "NOKPROBE_SYMBOL" is used for the
>>> symbol which is called in the kprobe processing path (e.g. x86 int3
>>> handler etc.).
>>>
>>> BTW, that the bug you reported is interesting. Even if another kprobe
>>> is called inside kprobe pre/post handler, it must be skipped.
>>> If you can share your kconfig, I can try to reproduce it.
>>>
>>> Thank you,
>>>
>>
>> Hi Masami,
>>
>> Thank you very much for your reply.
>>
>> Please use the attached config and diff file, here are the steps
>> to reproduce kernel hangs when kprobe memcpy on x86_64 fedora 36:
>>
>> (1) kernel 5.7
>> $ wget --no-check-certificate
>> https://git.kernel.org/torvalds/t/linux-5.7.tar.gz
>> $ ls
>> 5.7.config 5.7.diff linux-5.7.tar.gz
>> $ tar xf linux-5.7.tar.gz
>> $ cd linux-5.7/
>> $ patch -p1 < ../5.7.diff
>> $ cp ../5.7.config .config
>> $ make -j8
>> # make modules_install -j8
>> # make install
>> # set the default kernel and reboot
>> # modprobe kprobe_example symbol="memcpy"
>>
>> (2) kernel 6.2-rc1
>> $ wget --no-check-certificate
>> https://git.kernel.org/torvalds/t/linux-6.2-rc1.tar.gz
>> $ ls
>> 6.2-rc1.config 6.2-rc1.diff linux-6.2-rc1.tar.gz
>> $ tar xf linux-6.2-rc1.tar.gz
>> $ cd linux-6.2-rc1/
>> $ patch -p1 < ../6.2-rc1.diff
>> $ cp ../6.2-rc1.config .config
>> $ make -j8
>> # make modules_install -j8
>> # make install
>> # set the default kernel and reboot
>> # modprobe kprobe_example symbol="memcpy"
>>
>> By the way, I am developing and testing kprobe on LoongArch, I met the
>> same hang problems when probe some symbols, such as (1) handle_syscall,
>> like entry_SYSCALL_64 in arch/x86/entry/entry_64.S, used as syscall
>> exception handler, (2) memcpy, it may cause recursive exceptions like
>> x86.
>
> If you saw that without any change, please report it. At least
> memcpy is already marked as noinstr.

The current upstream mainline kernel has no problem, because it includes
commit e3a9e681adb7 ("x86/entry: Fixup bad_iret vs noinstr"), memcpy is
already marked as noinstr. But for the kernel without the above commit,
like kernel 5.7, it has problem.

>
>>
>> I do the following changes on LoongArch, could you please take a look
>> whether the code itself and the commit message are proper? Thank you.
>
> Yeah, this direction is good to me.
>

Thank you very much.

>>
>> LoongArch: Mark some assembler symbols as non-kprobe-able
>>
>> 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.
>>
>> diff --git a/arch/loongarch/include/asm/asm.h
>> b/arch/loongarch/include/asm/asm.h
>> index 40eea6aa469e..c51a0914fc99 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) \
>
> As same as other archs, can you rename it _ASM_NOKPROBE()?

OK, will modify it in the normal patch.

>
> Others look good to me.

Thanks,
Tiezhu

>
> Thanks!
>
>> + .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 d53b631c9022..05696bf7b69d 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 7c07d595ee89..b32dd25ce3a4 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)
>>
>> Thanks,
>> Tiezhu
>
>

2023-01-14 14:17:09

by Masami Hiramatsu

[permalink] [raw]
Subject: Re: kernel hangs when kprobe memcpy

On Sat, 14 Jan 2023 14:53:21 +0800
Tiezhu Yang <[email protected]> wrote:

>
>
> On 01/14/2023 01:38 PM, Masami Hiramatsu (Google) wrote:
> > Hi Tiezhu,
> >
> > On Fri, 13 Jan 2023 14:26:52 +0800
> > Tiezhu Yang <[email protected]> wrote:
> >
> >>
> >>
> >> On 01/12/2023 10:36 PM, Masami Hiramatsu (Google) wrote:
> >>> Hi Tiezhu,
> >>>
> >>> On Thu, 12 Jan 2023 21:32:51 +0800
> >>> Tiezhu Yang <[email protected]> wrote:
> >>>
> >>>>
> >>>>
> >>>> On 01/11/2023 07:38 PM, Tiezhu Yang wrote:
> >>>>> Hi all,
> >>>>>
> >>>>> (1) I have the following test environment, kernel hangs when kprobe memcpy:
> >>>>>
> >>>>> system: x86_64 fedora 36
> >>>>> kernel version: Linux 5.7 (compile and update)
> >>>>> test case: modprobe kprobe_example symbol="memcpy"
> >>>>> (CONFIG_SAMPLE_KPROBES=m)
> >>>>>
> >>>>> In order to fix build errors, it needs to unset CONFIG_NFP and do the
> >>>>> following changes:
> >>>>> commit 52a9dab6d892 ("libsubcmd: Fix use-after-free for realloc(..., 0)")
> >>>>> commit de979c83574a ("x86/entry: Build thunk_$(BITS) only if
> >>>>> CONFIG_PREEMPTION=y")
> >>>>>
> >>>>> (2) Using the latest upstream mainline kernel, no hang problem due to the
> >>>>> commit e3a9e681adb7 ("x86/entry: Fixup bad_iret vs noinstr") to prohibit
> >>>>> probing memcpy which is put into the .noinstr.text section.
> >>>>>
> >>>>> # modprobe kprobe_example symbol="memcpy"
> >>>>> modprobe: ERROR: could not insert 'kprobe_example': Invalid argument
> >>>>>
> >>>>> In my opinion, according to the commit message, the above commit is not
> >>>>> intended to fix the memcpy hang problem, the problem was fixed by accident.
> >>>>>
> >>>>> (3) If make handler_pre() and handler_post() as empty functions in the 5.7
> >>>>> kernel code, the above hang problem does not exist.
> >>>
> >>>
> >>>>>
> >>>>> diff --git a/samples/kprobes/kprobe_example.c
> >>>>> b/samples/kprobes/kprobe_example.c
> >>>>> index fd346f58ddba..c194171d8a46 100644
> >>>>> --- a/samples/kprobes/kprobe_example.c
> >>>>> +++ b/samples/kprobes/kprobe_example.c
> >>>>> @@ -28,8 +28,6 @@ static struct kprobe kp = {
> >>>>> static int __kprobes handler_pre(struct kprobe *p, struct pt_regs *regs)
> >>>>> {
> >>>>> #ifdef CONFIG_X86
> >>>>> - pr_info("<%s> p->addr = 0x%p, ip = %lx, flags = 0x%lx\n",
> >>>>> - p->symbol_name, p->addr, regs->ip, regs->flags);
> >>>>> #endif
> >>>>> #ifdef CONFIG_PPC
> >>>>> pr_info("<%s> p->addr = 0x%p, nip = 0x%lx, msr = 0x%lx\n",
> >>>>> @@ -65,8 +63,6 @@ static void __kprobes handler_post(struct kprobe *p,
> >>>>> struct pt_regs *regs,
> >>>>> unsigned long flags)
> >>>>> {
> >>>>> #ifdef CONFIG_X86
> >>>>> - pr_info("<%s> p->addr = 0x%p, flags = 0x%lx\n",
> >>>>> - p->symbol_name, p->addr, regs->flags);
> >>>>> #endif
> >>>>> #ifdef CONFIG_PPC
> >>>>> pr_info("<%s> p->addr = 0x%p, msr = 0x%lx\n",
> >>>>>
> >>>>> I want to know what is the real reason of the hang problem when kprobe
> >>>>> memcpy,
> >>>>> I guess it may be kprobe recursion, what do you think? Thank you.
> >>>>>
> >>>>> By the way, kprobe memset has no problem whether or not handler_pre() and
> >>>>> handler_post() are empty functions.
> >>>>>
> >>>>> Thanks,
> >>>>> Tiezhu
> >>>>
> >>>> I find out the following call trace:
> >>>>
> >>>> handler_pre()
> >>>> pr_info()
> >>>> printk()
> >>>> _printk()
> >>>> vprintk()
> >>>> vprintk_store()
> >>>> memcpy()
> >>>>
> >>>> I think it may cause recursive exceptions, so we should
> >>>> mark memcpy as non-kprobe-able, right?
> >>>
> >>> Yes, and the .noinstr.text (noinstr function attribute) is including
> >>> non-kprobe-able (nokprobe function attribute). I a function is nokprobe
> >>> and notrace, it should be noinstr. "NOKPROBE_SYMBOL" is used for the
> >>> symbol which is called in the kprobe processing path (e.g. x86 int3
> >>> handler etc.).
> >>>
> >>> BTW, that the bug you reported is interesting. Even if another kprobe
> >>> is called inside kprobe pre/post handler, it must be skipped.
> >>> If you can share your kconfig, I can try to reproduce it.
> >>>
> >>> Thank you,
> >>>
> >>
> >> Hi Masami,
> >>
> >> Thank you very much for your reply.
> >>
> >> Please use the attached config and diff file, here are the steps
> >> to reproduce kernel hangs when kprobe memcpy on x86_64 fedora 36:
> >>
> >> (1) kernel 5.7
> >> $ wget --no-check-certificate
> >> https://git.kernel.org/torvalds/t/linux-5.7.tar.gz
> >> $ ls
> >> 5.7.config 5.7.diff linux-5.7.tar.gz
> >> $ tar xf linux-5.7.tar.gz
> >> $ cd linux-5.7/
> >> $ patch -p1 < ../5.7.diff
> >> $ cp ../5.7.config .config
> >> $ make -j8
> >> # make modules_install -j8
> >> # make install
> >> # set the default kernel and reboot
> >> # modprobe kprobe_example symbol="memcpy"
> >>
> >> (2) kernel 6.2-rc1
> >> $ wget --no-check-certificate
> >> https://git.kernel.org/torvalds/t/linux-6.2-rc1.tar.gz
> >> $ ls
> >> 6.2-rc1.config 6.2-rc1.diff linux-6.2-rc1.tar.gz
> >> $ tar xf linux-6.2-rc1.tar.gz
> >> $ cd linux-6.2-rc1/
> >> $ patch -p1 < ../6.2-rc1.diff
> >> $ cp ../6.2-rc1.config .config
> >> $ make -j8
> >> # make modules_install -j8
> >> # make install
> >> # set the default kernel and reboot
> >> # modprobe kprobe_example symbol="memcpy"
> >>
> >> By the way, I am developing and testing kprobe on LoongArch, I met the
> >> same hang problems when probe some symbols, such as (1) handle_syscall,
> >> like entry_SYSCALL_64 in arch/x86/entry/entry_64.S, used as syscall
> >> exception handler, (2) memcpy, it may cause recursive exceptions like
> >> x86.
> >
> > If you saw that without any change, please report it. At least
> > memcpy is already marked as noinstr.
>
> The current upstream mainline kernel has no problem, because it includes
> commit e3a9e681adb7 ("x86/entry: Fixup bad_iret vs noinstr"), memcpy is
> already marked as noinstr. But for the kernel without the above commit,
> like kernel 5.7, it has problem.

Hmm, indeed. At least that should be backported to the stable kernels.
If we can reproduce it on 5.4 or older stable kernel, we have to backport
it or mark memcpy at least nokprobe.

Thank you,

>
> >
> >>
> >> I do the following changes on LoongArch, could you please take a look
> >> whether the code itself and the commit message are proper? Thank you.
> >
> > Yeah, this direction is good to me.
> >
>
> Thank you very much.
>
> >>
> >> LoongArch: Mark some assembler symbols as non-kprobe-able
> >>
> >> 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.
> >>
> >> diff --git a/arch/loongarch/include/asm/asm.h
> >> b/arch/loongarch/include/asm/asm.h
> >> index 40eea6aa469e..c51a0914fc99 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) \
> >
> > As same as other archs, can you rename it _ASM_NOKPROBE()?
>
> OK, will modify it in the normal patch.
>
> >
> > Others look good to me.
>
> Thanks,
> Tiezhu
>
> >
> > Thanks!
> >
> >> + .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 d53b631c9022..05696bf7b69d 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 7c07d595ee89..b32dd25ce3a4 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)
> >>
> >> Thanks,
> >> Tiezhu
> >
> >
>


--
Masami Hiramatsu (Google) <[email protected]>

2023-01-16 07:20:52

by Masami Hiramatsu

[permalink] [raw]
Subject: Re: kernel hangs when kprobe memcpy

Hi Tiezhu,

On Sat, 14 Jan 2023 14:53:21 +0800
Tiezhu Yang <[email protected]> wrote:

>
>
> On 01/14/2023 01:38 PM, Masami Hiramatsu (Google) wrote:
> > Hi Tiezhu,
> >
> > On Fri, 13 Jan 2023 14:26:52 +0800
> > Tiezhu Yang <[email protected]> wrote:
> >
> >>
> >>
> >> On 01/12/2023 10:36 PM, Masami Hiramatsu (Google) wrote:
> >>> Hi Tiezhu,
> >>>
> >>> On Thu, 12 Jan 2023 21:32:51 +0800
> >>> Tiezhu Yang <[email protected]> wrote:
> >>>
> >>>>
> >>>>
> >>>> On 01/11/2023 07:38 PM, Tiezhu Yang wrote:
> >>>>> Hi all,
> >>>>>
> >>>>> (1) I have the following test environment, kernel hangs when kprobe memcpy:
> >>>>>
> >>>>> system: x86_64 fedora 36
> >>>>> kernel version: Linux 5.7 (compile and update)
> >>>>> test case: modprobe kprobe_example symbol="memcpy"
> >>>>> (CONFIG_SAMPLE_KPROBES=m)
> >>>>>
> >>>>> In order to fix build errors, it needs to unset CONFIG_NFP and do the
> >>>>> following changes:
> >>>>> commit 52a9dab6d892 ("libsubcmd: Fix use-after-free for realloc(..., 0)")
> >>>>> commit de979c83574a ("x86/entry: Build thunk_$(BITS) only if
> >>>>> CONFIG_PREEMPTION=y")
> >>>>>
> >>>>> (2) Using the latest upstream mainline kernel, no hang problem due to the
> >>>>> commit e3a9e681adb7 ("x86/entry: Fixup bad_iret vs noinstr") to prohibit
> >>>>> probing memcpy which is put into the .noinstr.text section.
> >>>>>
> >>>>> # modprobe kprobe_example symbol="memcpy"
> >>>>> modprobe: ERROR: could not insert 'kprobe_example': Invalid argument
> >>>>>
> >>>>> In my opinion, according to the commit message, the above commit is not
> >>>>> intended to fix the memcpy hang problem, the problem was fixed by accident.
> >>>>>
> >>>>> (3) If make handler_pre() and handler_post() as empty functions in the 5.7
> >>>>> kernel code, the above hang problem does not exist.
> >>>
> >>>
> >>>>>
> >>>>> diff --git a/samples/kprobes/kprobe_example.c
> >>>>> b/samples/kprobes/kprobe_example.c
> >>>>> index fd346f58ddba..c194171d8a46 100644
> >>>>> --- a/samples/kprobes/kprobe_example.c
> >>>>> +++ b/samples/kprobes/kprobe_example.c
> >>>>> @@ -28,8 +28,6 @@ static struct kprobe kp = {
> >>>>> static int __kprobes handler_pre(struct kprobe *p, struct pt_regs *regs)
> >>>>> {
> >>>>> #ifdef CONFIG_X86
> >>>>> - pr_info("<%s> p->addr = 0x%p, ip = %lx, flags = 0x%lx\n",
> >>>>> - p->symbol_name, p->addr, regs->ip, regs->flags);
> >>>>> #endif
> >>>>> #ifdef CONFIG_PPC
> >>>>> pr_info("<%s> p->addr = 0x%p, nip = 0x%lx, msr = 0x%lx\n",
> >>>>> @@ -65,8 +63,6 @@ static void __kprobes handler_post(struct kprobe *p,
> >>>>> struct pt_regs *regs,
> >>>>> unsigned long flags)
> >>>>> {
> >>>>> #ifdef CONFIG_X86
> >>>>> - pr_info("<%s> p->addr = 0x%p, flags = 0x%lx\n",
> >>>>> - p->symbol_name, p->addr, regs->flags);
> >>>>> #endif
> >>>>> #ifdef CONFIG_PPC
> >>>>> pr_info("<%s> p->addr = 0x%p, msr = 0x%lx\n",
> >>>>>
> >>>>> I want to know what is the real reason of the hang problem when kprobe
> >>>>> memcpy,
> >>>>> I guess it may be kprobe recursion, what do you think? Thank you.
> >>>>>
> >>>>> By the way, kprobe memset has no problem whether or not handler_pre() and
> >>>>> handler_post() are empty functions.
> >>>>>
> >>>>> Thanks,
> >>>>> Tiezhu
> >>>>
> >>>> I find out the following call trace:
> >>>>
> >>>> handler_pre()
> >>>> pr_info()
> >>>> printk()
> >>>> _printk()
> >>>> vprintk()
> >>>> vprintk_store()
> >>>> memcpy()
> >>>>
> >>>> I think it may cause recursive exceptions, so we should
> >>>> mark memcpy as non-kprobe-able, right?
> >>>
> >>> Yes, and the .noinstr.text (noinstr function attribute) is including
> >>> non-kprobe-able (nokprobe function attribute). I a function is nokprobe
> >>> and notrace, it should be noinstr. "NOKPROBE_SYMBOL" is used for the
> >>> symbol which is called in the kprobe processing path (e.g. x86 int3
> >>> handler etc.).
> >>>
> >>> BTW, that the bug you reported is interesting. Even if another kprobe
> >>> is called inside kprobe pre/post handler, it must be skipped.
> >>> If you can share your kconfig, I can try to reproduce it.
> >>>
> >>> Thank you,
> >>>
> >>
> >> Hi Masami,
> >>
> >> Thank you very much for your reply.
> >>
> >> Please use the attached config and diff file, here are the steps
> >> to reproduce kernel hangs when kprobe memcpy on x86_64 fedora 36:
> >>
> >> (1) kernel 5.7
> >> $ wget --no-check-certificate
> >> https://git.kernel.org/torvalds/t/linux-5.7.tar.gz
> >> $ ls
> >> 5.7.config 5.7.diff linux-5.7.tar.gz
> >> $ tar xf linux-5.7.tar.gz
> >> $ cd linux-5.7/
> >> $ patch -p1 < ../5.7.diff
> >> $ cp ../5.7.config .config
> >> $ make -j8
> >> # make modules_install -j8
> >> # make install
> >> # set the default kernel and reboot
> >> # modprobe kprobe_example symbol="memcpy"
> >>
> >> (2) kernel 6.2-rc1
> >> $ wget --no-check-certificate
> >> https://git.kernel.org/torvalds/t/linux-6.2-rc1.tar.gz
> >> $ ls
> >> 6.2-rc1.config 6.2-rc1.diff linux-6.2-rc1.tar.gz
> >> $ tar xf linux-6.2-rc1.tar.gz
> >> $ cd linux-6.2-rc1/
> >> $ patch -p1 < ../6.2-rc1.diff
> >> $ cp ../6.2-rc1.config .config
> >> $ make -j8
> >> # make modules_install -j8
> >> # make install
> >> # set the default kernel and reboot
> >> # modprobe kprobe_example symbol="memcpy"
> >>
> >> By the way, I am developing and testing kprobe on LoongArch, I met the
> >> same hang problems when probe some symbols, such as (1) handle_syscall,
> >> like entry_SYSCALL_64 in arch/x86/entry/entry_64.S, used as syscall
> >> exception handler, (2) memcpy, it may cause recursive exceptions like
> >> x86.
> >
> > If you saw that without any change, please report it. At least
> > memcpy is already marked as noinstr.
>
> The current upstream mainline kernel has no problem, because it includes
> commit e3a9e681adb7 ("x86/entry: Fixup bad_iret vs noinstr"), memcpy is
> already marked as noinstr. But for the kernel without the above commit,
> like kernel 5.7, it has problem.

I've confirmed that kernel 5.4.228 (the latest stable tree) did not have
this issue (it already rejects the memcpy). Since 5.7 is not a stable
(maintained) tree, we can not send a patch to it.
Anyway, it is better to add a test case for the recursed probe function.
I made a patch below. Can your loongarch port pass this test case?

Thank you,

From: "Masami Hiramatsu (Google)" <[email protected]>
Date: Mon, 16 Jan 2023 15:19:52 +0900
Subject: [PATCH] test_kprobes: Add recursed kprobe test case

Add a recursed kprobe test case to the KUnit test module for kprobes.
This will probe a function which is called from the pre_handler and
post_handler itself. If the kprobe is correctly implemented, the recursed
kprobe handlers will be skipped and the number of skipped kprobe will
be counted on kprobe::nmissed.

Signed-off-by: Masami Hiramatsu (Google) <[email protected]>
---
lib/test_kprobes.c | 39 +++++++++++++++++++++++++++++++++++++--
1 file changed, 37 insertions(+), 2 deletions(-)

diff --git a/lib/test_kprobes.c b/lib/test_kprobes.c
index 1c95e5719802..0648f7154f5c 100644
--- a/lib/test_kprobes.c
+++ b/lib/test_kprobes.c
@@ -14,6 +14,7 @@

static u32 rand1, preh_val, posth_val;
static u32 (*target)(u32 value);
+static u32 (*recursed_target)(u32 value);
static u32 (*target2)(u32 value);
static struct kunit *current_test;

@@ -27,18 +28,27 @@ static noinline u32 kprobe_target(u32 value)
return (value / div_factor);
}

+static noinline u32 kprobe_recursed_target(u32 value)
+{
+ return (value / div_factor);
+}
+
static int kp_pre_handler(struct kprobe *p, struct pt_regs *regs)
{
KUNIT_EXPECT_FALSE(current_test, preemptible());
- preh_val = (rand1 / div_factor);
+
+ preh_val = recursed_target(rand1);
return 0;
}

static void kp_post_handler(struct kprobe *p, struct pt_regs *regs,
unsigned long flags)
{
+ u32 expval = recursed_target(rand1);
+
KUNIT_EXPECT_FALSE(current_test, preemptible());
- KUNIT_EXPECT_EQ(current_test, preh_val, (rand1 / div_factor));
+ KUNIT_EXPECT_EQ(current_test, preh_val, expval);
+
posth_val = preh_val + div_factor;
}

@@ -136,6 +146,29 @@ static void test_kprobes(struct kunit *test)
unregister_kprobes(kps, 2);
}

+static struct kprobe kp_missed = {
+ .symbol_name = "kprobe_recursed_target",
+ .pre_handler = kp_pre_handler,
+ .post_handler = kp_post_handler,
+};
+
+static void test_kprobe_missed(struct kunit *test)
+{
+ current_test = test;
+ preh_val = 0;
+ posth_val = 0;
+
+ KUNIT_EXPECT_EQ(test, 0, register_kprobe(&kp_missed));
+
+ recursed_target(rand1);
+
+ KUNIT_EXPECT_EQ(test, 2, kp_missed.nmissed);
+ KUNIT_EXPECT_NE(test, 0, preh_val);
+ KUNIT_EXPECT_NE(test, 0, posth_val);
+
+ unregister_kprobe(&kp_missed);
+}
+
#ifdef CONFIG_KRETPROBES
static u32 krph_val;

@@ -336,6 +369,7 @@ static int kprobes_test_init(struct kunit *test)
{
target = kprobe_target;
target2 = kprobe_target2;
+ recursed_target = kprobe_recursed_target;
stacktrace_target = kprobe_stacktrace_target;
internal_target = kprobe_stacktrace_internal_target;
stacktrace_driver = kprobe_stacktrace_driver;
@@ -346,6 +380,7 @@ static int kprobes_test_init(struct kunit *test)
static struct kunit_case kprobes_testcases[] = {
KUNIT_CASE(test_kprobe),
KUNIT_CASE(test_kprobes),
+ KUNIT_CASE(test_kprobe_missed),
#ifdef CONFIG_KRETPROBES
KUNIT_CASE(test_kretprobe),
KUNIT_CASE(test_kretprobes),
--
2.39.0.246.g2a6d74b583-goog


--
Masami Hiramatsu (Google) <[email protected]>

2023-01-16 13:37:42

by Tiezhu Yang

[permalink] [raw]
Subject: Re: kernel hangs when kprobe memcpy



On 01/16/2023 02:41 PM, Masami Hiramatsu (Google) wrote:
> Hi Tiezhu,
>
> On Sat, 14 Jan 2023 14:53:21 +0800
> Tiezhu Yang <[email protected]> wrote:
>
>>
>>
>> On 01/14/2023 01:38 PM, Masami Hiramatsu (Google) wrote:
>>> Hi Tiezhu,
>>>
>>> On Fri, 13 Jan 2023 14:26:52 +0800
>>> Tiezhu Yang <[email protected]> wrote:
>>>
>>>>
>>>>
>>>> On 01/12/2023 10:36 PM, Masami Hiramatsu (Google) wrote:
>>>>> Hi Tiezhu,
>>>>>
>>>>> On Thu, 12 Jan 2023 21:32:51 +0800
>>>>> Tiezhu Yang <[email protected]> wrote:
>>>>>
>>>>>>
>>>>>>
>>>>>> On 01/11/2023 07:38 PM, Tiezhu Yang wrote:
>>>>>>> Hi all,
>>>>>>>
>>>>>>> (1) I have the following test environment, kernel hangs when kprobe memcpy:
>>>>>>>
>>>>>>> system: x86_64 fedora 36
>>>>>>> kernel version: Linux 5.7 (compile and update)
>>>>>>> test case: modprobe kprobe_example symbol="memcpy"
>>>>>>> (CONFIG_SAMPLE_KPROBES=m)
>>>>>>>
>>>>>>> In order to fix build errors, it needs to unset CONFIG_NFP and do the
>>>>>>> following changes:
>>>>>>> commit 52a9dab6d892 ("libsubcmd: Fix use-after-free for realloc(..., 0)")
>>>>>>> commit de979c83574a ("x86/entry: Build thunk_$(BITS) only if
>>>>>>> CONFIG_PREEMPTION=y")
>>>>>>>
>>>>>>> (2) Using the latest upstream mainline kernel, no hang problem due to the
>>>>>>> commit e3a9e681adb7 ("x86/entry: Fixup bad_iret vs noinstr") to prohibit
>>>>>>> probing memcpy which is put into the .noinstr.text section.
>>>>>>>
>>>>>>> # modprobe kprobe_example symbol="memcpy"
>>>>>>> modprobe: ERROR: could not insert 'kprobe_example': Invalid argument
>>>>>>>
>>>>>>> In my opinion, according to the commit message, the above commit is not
>>>>>>> intended to fix the memcpy hang problem, the problem was fixed by accident.
>>>>>>>
>>>>>>> (3) If make handler_pre() and handler_post() as empty functions in the 5.7
>>>>>>> kernel code, the above hang problem does not exist.
>>>>>
>>>>>
>>>>>>>
>>>>>>> diff --git a/samples/kprobes/kprobe_example.c
>>>>>>> b/samples/kprobes/kprobe_example.c
>>>>>>> index fd346f58ddba..c194171d8a46 100644
>>>>>>> --- a/samples/kprobes/kprobe_example.c
>>>>>>> +++ b/samples/kprobes/kprobe_example.c
>>>>>>> @@ -28,8 +28,6 @@ static struct kprobe kp = {
>>>>>>> static int __kprobes handler_pre(struct kprobe *p, struct pt_regs *regs)
>>>>>>> {
>>>>>>> #ifdef CONFIG_X86
>>>>>>> - pr_info("<%s> p->addr = 0x%p, ip = %lx, flags = 0x%lx\n",
>>>>>>> - p->symbol_name, p->addr, regs->ip, regs->flags);
>>>>>>> #endif
>>>>>>> #ifdef CONFIG_PPC
>>>>>>> pr_info("<%s> p->addr = 0x%p, nip = 0x%lx, msr = 0x%lx\n",
>>>>>>> @@ -65,8 +63,6 @@ static void __kprobes handler_post(struct kprobe *p,
>>>>>>> struct pt_regs *regs,
>>>>>>> unsigned long flags)
>>>>>>> {
>>>>>>> #ifdef CONFIG_X86
>>>>>>> - pr_info("<%s> p->addr = 0x%p, flags = 0x%lx\n",
>>>>>>> - p->symbol_name, p->addr, regs->flags);
>>>>>>> #endif
>>>>>>> #ifdef CONFIG_PPC
>>>>>>> pr_info("<%s> p->addr = 0x%p, msr = 0x%lx\n",
>>>>>>>
>>>>>>> I want to know what is the real reason of the hang problem when kprobe
>>>>>>> memcpy,
>>>>>>> I guess it may be kprobe recursion, what do you think? Thank you.
>>>>>>>
>>>>>>> By the way, kprobe memset has no problem whether or not handler_pre() and
>>>>>>> handler_post() are empty functions.
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Tiezhu
>>>>>>
>>>>>> I find out the following call trace:
>>>>>>
>>>>>> handler_pre()
>>>>>> pr_info()
>>>>>> printk()
>>>>>> _printk()
>>>>>> vprintk()
>>>>>> vprintk_store()
>>>>>> memcpy()
>>>>>>
>>>>>> I think it may cause recursive exceptions, so we should
>>>>>> mark memcpy as non-kprobe-able, right?
>>>>>
>>>>> Yes, and the .noinstr.text (noinstr function attribute) is including
>>>>> non-kprobe-able (nokprobe function attribute). I a function is nokprobe
>>>>> and notrace, it should be noinstr. "NOKPROBE_SYMBOL" is used for the
>>>>> symbol which is called in the kprobe processing path (e.g. x86 int3
>>>>> handler etc.).
>>>>>
>>>>> BTW, that the bug you reported is interesting. Even if another kprobe
>>>>> is called inside kprobe pre/post handler, it must be skipped.
>>>>> If you can share your kconfig, I can try to reproduce it.
>>>>>
>>>>> Thank you,
>>>>>
>>>>
>>>> Hi Masami,
>>>>
>>>> Thank you very much for your reply.
>>>>
>>>> Please use the attached config and diff file, here are the steps
>>>> to reproduce kernel hangs when kprobe memcpy on x86_64 fedora 36:
>>>>
>>>> (1) kernel 5.7
>>>> $ wget --no-check-certificate
>>>> https://git.kernel.org/torvalds/t/linux-5.7.tar.gz
>>>> $ ls
>>>> 5.7.config 5.7.diff linux-5.7.tar.gz
>>>> $ tar xf linux-5.7.tar.gz
>>>> $ cd linux-5.7/
>>>> $ patch -p1 < ../5.7.diff
>>>> $ cp ../5.7.config .config
>>>> $ make -j8
>>>> # make modules_install -j8
>>>> # make install
>>>> # set the default kernel and reboot
>>>> # modprobe kprobe_example symbol="memcpy"
>>>>
>>>> (2) kernel 6.2-rc1
>>>> $ wget --no-check-certificate
>>>> https://git.kernel.org/torvalds/t/linux-6.2-rc1.tar.gz
>>>> $ ls
>>>> 6.2-rc1.config 6.2-rc1.diff linux-6.2-rc1.tar.gz
>>>> $ tar xf linux-6.2-rc1.tar.gz
>>>> $ cd linux-6.2-rc1/
>>>> $ patch -p1 < ../6.2-rc1.diff
>>>> $ cp ../6.2-rc1.config .config
>>>> $ make -j8
>>>> # make modules_install -j8
>>>> # make install
>>>> # set the default kernel and reboot
>>>> # modprobe kprobe_example symbol="memcpy"
>>>>
>>>> By the way, I am developing and testing kprobe on LoongArch, I met the
>>>> same hang problems when probe some symbols, such as (1) handle_syscall,
>>>> like entry_SYSCALL_64 in arch/x86/entry/entry_64.S, used as syscall
>>>> exception handler, (2) memcpy, it may cause recursive exceptions like
>>>> x86.
>>>
>>> If you saw that without any change, please report it. At least
>>> memcpy is already marked as noinstr.
>>
>> The current upstream mainline kernel has no problem, because it includes
>> commit e3a9e681adb7 ("x86/entry: Fixup bad_iret vs noinstr"), memcpy is
>> already marked as noinstr. But for the kernel without the above commit,
>> like kernel 5.7, it has problem.
>
> I've confirmed that kernel 5.4.228 (the latest stable tree) did not have
> this issue (it already rejects the memcpy). Since 5.7 is not a stable
> (maintained) tree, we can not send a patch to it.
> Anyway, it is better to add a test case for the recursed probe function.
> I made a patch below. Can your loongarch port pass this test case?

Yes, test_kprobes passed on LoongArch.

Without this patch:

[ 58.536879] KTAP version 1
[ 58.536892] # Subtest: kprobes_test
[ 58.536895] 1..4
[ 58.550235] ok 1 test_kprobe
[ 58.574234] ok 2 test_kprobes
[ 58.606237] ok 3 test_kretprobe
[ 58.630234] ok 4 test_kretprobes
[ 58.630237] # kprobes_test: pass:4 fail:0 skip:0 total:4
[ 58.630240] # Totals: pass:4 fail:0 skip:0 total:4
[ 58.630242] ok 1 kprobes_test

With this patch:

[ 21.800697] KTAP version 1
[ 21.800710] # Subtest: kprobes_test
[ 21.800713] 1..5
[ 21.818189] ok 1 test_kprobe
[ 21.838188] ok 2 test_kprobes
[ 21.858189] ok 3 test_kprobe_missed
[ 21.878190] ok 4 test_kretprobe
[ 21.898189] ok 5 test_kretprobes
[ 21.898192] # kprobes_test: pass:5 fail:0 skip:0 total:5
[ 21.898195] # Totals: pass:5 fail:0 skip:0 total:5
[ 21.898197] ok 1 kprobes_test

Thanks,
Tiezhu

>
> Thank you,
>
> From: "Masami Hiramatsu (Google)" <[email protected]>
> Date: Mon, 16 Jan 2023 15:19:52 +0900
> Subject: [PATCH] test_kprobes: Add recursed kprobe test case
>
> Add a recursed kprobe test case to the KUnit test module for kprobes.
> This will probe a function which is called from the pre_handler and
> post_handler itself. If the kprobe is correctly implemented, the recursed
> kprobe handlers will be skipped and the number of skipped kprobe will
> be counted on kprobe::nmissed.
>
> Signed-off-by: Masami Hiramatsu (Google) <[email protected]>
> ---
> lib/test_kprobes.c | 39 +++++++++++++++++++++++++++++++++++++--
> 1 file changed, 37 insertions(+), 2 deletions(-)
>
> diff --git a/lib/test_kprobes.c b/lib/test_kprobes.c
> index 1c95e5719802..0648f7154f5c 100644
> --- a/lib/test_kprobes.c
> +++ b/lib/test_kprobes.c
> @@ -14,6 +14,7 @@
>
> static u32 rand1, preh_val, posth_val;
> static u32 (*target)(u32 value);
> +static u32 (*recursed_target)(u32 value);
> static u32 (*target2)(u32 value);
> static struct kunit *current_test;
>
> @@ -27,18 +28,27 @@ static noinline u32 kprobe_target(u32 value)
> return (value / div_factor);
> }
>
> +static noinline u32 kprobe_recursed_target(u32 value)
> +{
> + return (value / div_factor);
> +}
> +
> static int kp_pre_handler(struct kprobe *p, struct pt_regs *regs)
> {
> KUNIT_EXPECT_FALSE(current_test, preemptible());
> - preh_val = (rand1 / div_factor);
> +
> + preh_val = recursed_target(rand1);
> return 0;
> }
>
> static void kp_post_handler(struct kprobe *p, struct pt_regs *regs,
> unsigned long flags)
> {
> + u32 expval = recursed_target(rand1);
> +
> KUNIT_EXPECT_FALSE(current_test, preemptible());
> - KUNIT_EXPECT_EQ(current_test, preh_val, (rand1 / div_factor));
> + KUNIT_EXPECT_EQ(current_test, preh_val, expval);
> +
> posth_val = preh_val + div_factor;
> }
>
> @@ -136,6 +146,29 @@ static void test_kprobes(struct kunit *test)
> unregister_kprobes(kps, 2);
> }
>
> +static struct kprobe kp_missed = {
> + .symbol_name = "kprobe_recursed_target",
> + .pre_handler = kp_pre_handler,
> + .post_handler = kp_post_handler,
> +};
> +
> +static void test_kprobe_missed(struct kunit *test)
> +{
> + current_test = test;
> + preh_val = 0;
> + posth_val = 0;
> +
> + KUNIT_EXPECT_EQ(test, 0, register_kprobe(&kp_missed));
> +
> + recursed_target(rand1);
> +
> + KUNIT_EXPECT_EQ(test, 2, kp_missed.nmissed);
> + KUNIT_EXPECT_NE(test, 0, preh_val);
> + KUNIT_EXPECT_NE(test, 0, posth_val);
> +
> + unregister_kprobe(&kp_missed);
> +}
> +
> #ifdef CONFIG_KRETPROBES
> static u32 krph_val;
>
> @@ -336,6 +369,7 @@ static int kprobes_test_init(struct kunit *test)
> {
> target = kprobe_target;
> target2 = kprobe_target2;
> + recursed_target = kprobe_recursed_target;
> stacktrace_target = kprobe_stacktrace_target;
> internal_target = kprobe_stacktrace_internal_target;
> stacktrace_driver = kprobe_stacktrace_driver;
> @@ -346,6 +380,7 @@ static int kprobes_test_init(struct kunit *test)
> static struct kunit_case kprobes_testcases[] = {
> KUNIT_CASE(test_kprobe),
> KUNIT_CASE(test_kprobes),
> + KUNIT_CASE(test_kprobe_missed),
> #ifdef CONFIG_KRETPROBES
> KUNIT_CASE(test_kretprobe),
> KUNIT_CASE(test_kretprobes),
>

2023-01-31 03:38:59

by Tiezhu Yang

[permalink] [raw]
Subject: Re: kernel hangs when kprobe memcpy



On 01/16/2023 02:41 PM, Masami Hiramatsu (Google) wrote:
> Hi Tiezhu,
>
> On Sat, 14 Jan 2023 14:53:21 +0800
> Tiezhu Yang <[email protected]> wrote:

...

>>> If you saw that without any change, please report it. At least
>>> memcpy is already marked as noinstr.
>>
>> The current upstream mainline kernel has no problem, because it includes
>> commit e3a9e681adb7 ("x86/entry: Fixup bad_iret vs noinstr"), memcpy is
>> already marked as noinstr. But for the kernel without the above commit,
>> like kernel 5.7, it has problem.
>
> I've confirmed that kernel 5.4.228 (the latest stable tree) did not have
> this issue (it already rejects the memcpy).

I just tested the stable kernel 5.4.230 on x86_64 fedora 36,
without any change, kernel hangs when execute cmd:
modprobe kprobe_example symbol="memcpy"

https://cdn.kernel.org/pub/linux/kernel/v5.x/linux-5.4.230.tar.xz

Am I missing something? Since 5.4 is a stable tree, should we do
something to fix it?

Thanks,
Tiezhu