From: Guo Ren <guoren@kernel.org>
To: Masami Hiramatsu <mhiramat@kernel.org>
Cc: "Palmer Dabbelt" <palmerdabbelt@google.com>,
"Paul Walmsley" <paul.walmsley@sifive.com>,
"Anup Patel" <anup@brainfault.org>,
"Greentime Hu" <greentime.hu@sifive.com>,
"Zong Li" <zong.li@sifive.com>, "Patrick Stählin" <me@packi.ch>,
"Björn Töpel" <bjorn.topel@gmail.com>,
"Atish Patra" <atish.patra@wdc.com>,
penberg@kernel.org, linux-riscv <linux-riscv@lists.infradead.org>,
"Linux Kernel Mailing List" <linux-kernel@vger.kernel.org>,
linux-csky@vger.kernel.org, "Guo Ren" <guoren@linux.alibaba.com>
Subject: Re: [PATCH v2 6/6] riscv: Add KPROBES_ON_FTRACE supported
Date: Sat, 11 Jul 2020 09:32:01 +0800 [thread overview]
Message-ID: <CAJF2gTSfFY6qf7gZ9t80P-3cACWi3oEe4X8ek+_1nQZZT3Uk5w@mail.gmail.com> (raw)
In-Reply-To: <20200710225017.5ce329485e911f99e17cd483@kernel.org>
Thx Masami,
On Fri, Jul 10, 2020 at 9:50 PM Masami Hiramatsu <mhiramat@kernel.org> wrote:
>
> Hi Guo,
>
> On Thu, 9 Jul 2020 02:19:14 +0000
> guoren@kernel.org wrote:
>
> > +/* Ftrace callback handler for kprobes -- called under preepmt disabed */
> > +void kprobe_ftrace_handler(unsigned long ip, unsigned long parent_ip,
> > + struct ftrace_ops *ops, struct pt_regs *regs)
> > +{
> > + struct kprobe *p;
> > + struct kprobe_ctlblk *kcb;
> > +
> > + p = get_kprobe((kprobe_opcode_t *)ip);
> > + if (unlikely(!p) || kprobe_disabled(p))
> > + return;
> > +
> > + kcb = get_kprobe_ctlblk();
> > + if (kprobe_running()) {
> > + kprobes_inc_nmissed_count(p);
> > + } else {
> > + /*
> > + * The regs->epc hasn't been saved by SAVE_ALL in mcount-dyn.S
> > + * So no need to resume it, just for kprobe handler.
> > + */
> > + instruction_pointer_set(regs, ip);
> > + __this_cpu_write(current_kprobe, p);
> > + kcb->kprobe_status = KPROBE_HIT_ACTIVE;
> > + if (!p->pre_handler || !p->pre_handler(p, regs)) {
> > + /*
> > + * Emulate singlestep (and also recover regs->pc)
> > + * as if there is a nop
> > + */
> > + instruction_pointer_set(regs,
> > + (unsigned long)p->addr + MCOUNT_INSN_SIZE);
> > + if (unlikely(p->post_handler)) {
> > + kcb->kprobe_status = KPROBE_HIT_SSDONE;
> > + p->post_handler(p, regs, 0);
> > + }
>
> Hmm, don't you need restoring the previous instruction pointer here?
look at riscv mcount-dyn.S SAVE_ALL function, sp frame lay out like this:
-----------------------
| return address |
-----------------------
| frame pointer |
-----------------------
| pt_regs x1-x31|
-----------------------
It's not a complete pt_regs for the handler, so modifing regs->ip is no use.
> If you don't support modifying the instruction pointer in the handler,
We can modify ip like this if necessary:
*(unsigned long *)((unsigned long)regs + sizeof(struct pt_regs) + 8) = xxx;
> it must not be compatible with kprobes.
Why, can you show related codes? thank you very much.
>
> Now BPF function override and function error injection depends on
> this behevior, so could you consider to support it in the "ftrace"
> implementation at first? (And if it is enabled, you can enable the
> livepatch on RISCV too)
Great message!
But can you show me codes that bpf and err-jnject using the behavior? Thx
I'll try to fix up it :)
--
Best Regards
Guo Ren
ML: https://lore.kernel.org/linux-csky/
next prev parent reply other threads:[~2020-07-11 1:32 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-07-09 2:19 [PATCH v2 0/6] riscv: Add k/uprobe supported guoren
2020-07-09 2:19 ` [PATCH v2 1/6] riscv: Fixup __vdso_gettimeofday broke dynamic ftrace guoren
2020-07-21 1:10 ` Palmer Dabbelt
2020-07-09 2:19 ` [PATCH v2 2/6] RISC-V: Implement ptrace regs and stack API guoren
2020-07-09 2:19 ` [PATCH v2 3/6] riscv: Fixup compile error BUILD_BUG_ON failed guoren
2020-07-09 2:19 ` [PATCH v2 4/6] riscv: Add kprobes supported guoren
2020-07-09 2:19 ` [PATCH v2 5/6] riscv: Add uprobes supported guoren
2020-07-09 2:19 ` [PATCH v2 6/6] riscv: Add KPROBES_ON_FTRACE supported guoren
2020-07-10 13:50 ` Masami Hiramatsu
2020-07-11 1:32 ` Guo Ren [this message]
2020-07-12 13:37 ` Masami Hiramatsu
2020-07-13 23:47 ` Guo Ren
2020-07-14 11:32 ` Masami Hiramatsu
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAJF2gTSfFY6qf7gZ9t80P-3cACWi3oEe4X8ek+_1nQZZT3Uk5w@mail.gmail.com \
--to=guoren@kernel.org \
--cc=anup@brainfault.org \
--cc=atish.patra@wdc.com \
--cc=bjorn.topel@gmail.com \
--cc=greentime.hu@sifive.com \
--cc=guoren@linux.alibaba.com \
--cc=linux-csky@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-riscv@lists.infradead.org \
--cc=me@packi.ch \
--cc=mhiramat@kernel.org \
--cc=palmerdabbelt@google.com \
--cc=paul.walmsley@sifive.com \
--cc=penberg@kernel.org \
--cc=zong.li@sifive.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).