linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Re: [Phishing Risk] [PATCH] kprobe: fix kretprobe stack backtrace
       [not found] <20210625084748.18128-1-wangqiang.wq.frank@bytedance.com>
@ 2021-06-25 10:24 ` Muchun Song
  2021-06-28 13:34   ` Masami Hiramatsu
  0 siblings, 1 reply; 2+ messages in thread
From: Muchun Song @ 2021-06-25 10:24 UTC (permalink / raw)
  To: Qiang Wang
  Cc: Naveen N . Rao, Anil S Keshavamurthy, David Miller,
	Masami Hiramatsu, Alexei Starovoitov, Daniel Borkmann,
	Andrii Nakryiko, Martin KaFai Lau, Song Liu, Yonghong Song,
	John Fastabend, kpsingh, LKML, Networking, bpf, Chengming Zhou,
	Xiongchun duan

On Fri, Jun 25, 2021 at 4:49 PM Qiang Wang
<wangqiang.wq.frank@bytedance.com> wrote:
>
> We found that we couldn't get the correct kernel stack from
> kretprobe. For example:
>
> bpftrace -e 'kr:submit_bio {print(kstack)}'
> Attaching 1 probe...
>
>         kretprobe_trampoline+0
>
>         kretprobe_trampoline+0
>
> The problem is caused by the wrong instruction register which
> points to the address of kretprobe_trampoline in regs.
> So we set the real return address in instruction register.
> Finally, we tested and successfully fixed it.
>
> bpftrace -e 'kr:submit_bio {print(kstack)}'
> Attaching 1 probe...
>
>         ext4_mpage_readpages+475
>         read_pages+139
>         page_cache_ra_unbounded+417
>         filemap_get_pages+245
>         filemap_read+169
>         __kernel_read+327
>         bprm_execve+648
>         do_execveat_common.isra.39+409
>         __x64_sys_execve+50
>         do_syscall_64+54
>         entry_SYSCALL_64_after_hwframe+68
>
> Reported-by: Chengming Zhou <zhouchengming@bytedance.com>
> Signed-off-by: Qiang Wang <wangqiang.wq.frank@bytedance.com>

Seems like a bug. Maybe we should add a "Fixes" tag here.

> ---
>  kernel/kprobes.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/kernel/kprobes.c b/kernel/kprobes.c
> index 745f08fdd..1130381ca 100644
> --- a/kernel/kprobes.c
> +++ b/kernel/kprobes.c
> @@ -1899,6 +1899,9 @@ unsigned long __kretprobe_trampoline_handler(struct pt_regs *regs,
>         current->kretprobe_instances.first = node->next;
>         node->next = NULL;
>
> +       /* Kretprobe handler expects address is the real return address */
> +       instruction_pointer_set(regs, (unsigned long)correct_ret_addr);
> +
>         /* Run them..  */
>         while (first) {
>                 ri = container_of(first, struct kretprobe_instance, llist);
> --
> 2.20.1
>

^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [Phishing Risk] [PATCH] kprobe: fix kretprobe stack backtrace
  2021-06-25 10:24 ` [Phishing Risk] [PATCH] kprobe: fix kretprobe stack backtrace Muchun Song
@ 2021-06-28 13:34   ` Masami Hiramatsu
  0 siblings, 0 replies; 2+ messages in thread
From: Masami Hiramatsu @ 2021-06-28 13:34 UTC (permalink / raw)
  To: Muchun Song
  Cc: Qiang Wang, Naveen N . Rao, Anil S Keshavamurthy, David Miller,
	Masami Hiramatsu, Alexei Starovoitov, Daniel Borkmann,
	Andrii Nakryiko, Martin KaFai Lau, Song Liu, Yonghong Song,
	John Fastabend, kpsingh, LKML, Networking, bpf, Chengming Zhou,
	Xiongchun duan

On Fri, 25 Jun 2021 18:24:12 +0800
Muchun Song <songmuchun@bytedance.com> wrote:

> On Fri, Jun 25, 2021 at 4:49 PM Qiang Wang
> <wangqiang.wq.frank@bytedance.com> wrote:
> >
> > We found that we couldn't get the correct kernel stack from
> > kretprobe. For example:
> >
> > bpftrace -e 'kr:submit_bio {print(kstack)}'
> > Attaching 1 probe...
> >
> >         kretprobe_trampoline+0
> >
> >         kretprobe_trampoline+0
> >
> > The problem is caused by the wrong instruction register which
> > points to the address of kretprobe_trampoline in regs.
> > So we set the real return address in instruction register.
> > Finally, we tested and successfully fixed it.
> >
> > bpftrace -e 'kr:submit_bio {print(kstack)}'
> > Attaching 1 probe...
> >
> >         ext4_mpage_readpages+475
> >         read_pages+139
> >         page_cache_ra_unbounded+417
> >         filemap_get_pages+245
> >         filemap_read+169
> >         __kernel_read+327
> >         bprm_execve+648
> >         do_execveat_common.isra.39+409
> >         __x64_sys_execve+50
> >         do_syscall_64+54
> >         entry_SYSCALL_64_after_hwframe+68
> >
> > Reported-by: Chengming Zhou <zhouchengming@bytedance.com>
> > Signed-off-by: Qiang Wang <wangqiang.wq.frank@bytedance.com>
> 
> Seems like a bug. Maybe we should add a "Fixes" tag here.

No, that is not a bug in the kretprobes. If you carefully check
kretprobes provided the rp->addr as ip address. BPF just did not
use it.

Anyway, I already made a same patch in the below series.

https://lore.kernel.org/bpf/162400000592.506599.4695807810528866713.stgit@devnote2/

and you can see that the series is including below 3 patches for that change.

https://lore.kernel.org/bpf/162399997853.506599.13701157683968161733.stgit@devnote2/
https://lore.kernel.org/bpf/162399998747.506599.1115560529431673586.stgit@devnote2/
https://lore.kernel.org/bpf/162399999702.506599.16339931387573094059.stgit@devnote2/

Without these patches, this change will break other arch.

Thanks,

> 
> > ---
> >  kernel/kprobes.c | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/kernel/kprobes.c b/kernel/kprobes.c
> > index 745f08fdd..1130381ca 100644
> > --- a/kernel/kprobes.c
> > +++ b/kernel/kprobes.c
> > @@ -1899,6 +1899,9 @@ unsigned long __kretprobe_trampoline_handler(struct pt_regs *regs,
> >         current->kretprobe_instances.first = node->next;
> >         node->next = NULL;
> >
> > +       /* Kretprobe handler expects address is the real return address */
> > +       instruction_pointer_set(regs, (unsigned long)correct_ret_addr);
> > +
> >         /* Run them..  */
> >         while (first) {
> >                 ri = container_of(first, struct kretprobe_instance, llist);
> > --
> > 2.20.1
> >


-- 
Masami Hiramatsu <mhiramat@kernel.org>

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2021-06-28 13:35 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <20210625084748.18128-1-wangqiang.wq.frank@bytedance.com>
2021-06-25 10:24 ` [Phishing Risk] [PATCH] kprobe: fix kretprobe stack backtrace Muchun Song
2021-06-28 13:34   ` Masami Hiramatsu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).