linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: Masami Hiramatsu <mhiramat@kernel.org>
To: Steven Rostedt <rostedt@goodmis.org>
Cc: Mark Rutland <mark.rutland@arm.com>,
	Wang ShaoBo <bobo.shaobowang@huawei.com>,
	cj.chengjian@huawei.com, huawei.libin@huawei.com,
	xiexiuqi@huawei.com, liwei391@huawei.com,
	linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org, catalin.marinas@arm.com,
	will@kernel.org, zengshun.wu@outlook.com,
	Jiri Olsa <jolsa@kernel.org>
Subject: Re: [RFC PATCH -next v2 3/4] arm64/ftrace: support dynamically allocated trampolines
Date: Thu, 12 May 2022 21:02:31 +0900	[thread overview]
Message-ID: <20220512210231.f9178a98f20a37981b1e89e3@kernel.org> (raw)
In-Reply-To: <20220511111207.25d1a693@gandalf.local.home>

On Wed, 11 May 2022 11:12:07 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:

> On Wed, 11 May 2022 23:34:50 +0900
> Masami Hiramatsu <mhiramat@kernel.org> wrote:
> 
> > OK, so fregs::regs will have a subset of pt_regs, and accessibility of
> > the registers depends on the architecture. If we can have a checker like
> > 
> > ftrace_regs_exist(fregs, reg_offset)
> 
> Or something. I'd have to see the use case.
> 
> > 
> > kprobe on ftrace or fprobe user (BPF) can filter user's requests.
> > I think I can introduce a flag for kprobes so that user can make a
> > kprobe handler only using a subset of registers. 
> > Maybe similar filter code is also needed for BPF 'user space' library
> > because this check must be done when compiling BPF.
> 
> Is there any other case without full regs that the user would want anything
> other than the args, stack pointer and instruction pointer?

For the kprobes APIs/events, yes, it needs to access to the registers
which is used for local variables when probing inside the function body.
However at the function entry, I think almost no use case. (BTW, pstate
is a bit special, that may show the actual processor-level status
(context), so for the debugging, user might want to read it.)

Thus the BPF use case via fprobes, I think there is no usecase.
My concern is that the BPF may allow user program to access any
field of pt_regs. Thus if the user miss-programmed, they may see
a wrong value (I guess the fregs is not zero-filled) for unsaved
registers.

> That is, have a flag that says "only_args" or something, that says they
> will only get the registers for arguments, a stack pointer, and the
> instruction pointer (note, the fregs may not have the instruction pointer
> as that is passed to the the caller via the "ip" parameter. If the fregs
> needs that, we can add a "ftrace_regs_set_ip()" before calling the
> callback registered to the fprobe).

Yes, that is what I'm thinking. If "only_args" flag is set, BPF runtime
must check the user program. And if it finds the program access the
unsaved registers, it should stop executing.

BTW, "what register is saved" can be determined statically, thus I think
we just need the offset for checking (for fprobe usecase, since it will
set the ftrace_ops flag by itself.)


Thank you,

-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  reply	other threads:[~2022-05-12 12:03 UTC|newest]

Thread overview: 44+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-03-16 10:01 [RFC PATCH -next v2 0/4] arm64/ftrace: support dynamic trampoline Wang ShaoBo
2022-03-16 10:01 ` [RFC PATCH -next v2 1/4] arm64: introduce aarch64_insn_gen_load_literal Wang ShaoBo
2022-03-16 10:01 ` [RFC PATCH -next v2 2/4] arm64/ftrace: introduce ftrace dynamic trampoline entrances Wang ShaoBo
2022-03-16 10:01 ` [RFC PATCH -next v2 3/4] arm64/ftrace: support dynamically allocated trampolines Wang ShaoBo
2022-04-21 13:10   ` Mark Rutland
2022-04-21 14:06     ` Steven Rostedt
2022-04-21 14:08       ` Steven Rostedt
2022-04-21 15:14       ` Mark Rutland
2022-04-21 15:42         ` Steven Rostedt
2022-04-21 16:27           ` Mark Rutland
2022-04-21 17:06             ` Steven Rostedt
2022-04-22 10:12               ` Mark Rutland
2022-04-22 15:45                 ` Steven Rostedt
2022-04-22 17:27                   ` Mark Rutland
2022-04-26  8:47                     ` Masami Hiramatsu
2022-05-04 10:24                       ` Mark Rutland
2022-05-05  3:15                         ` Masami Hiramatsu
2022-05-09 18:22                           ` Steven Rostedt
2022-05-10  9:10                             ` Masami Hiramatsu
2022-05-10 14:44                               ` Steven Rostedt
2022-05-11 14:34                                 ` Masami Hiramatsu
2022-05-11 15:12                                   ` Steven Rostedt
2022-05-12 12:02                                     ` Masami Hiramatsu [this message]
2022-05-12 13:50                                       ` Steven Rostedt
2022-05-25 12:17                                       ` Mark Rutland
2022-05-25 13:43                                         ` Steven Rostedt
2022-05-25 17:12                                           ` Mark Rutland
2022-05-30  1:03                                         ` Masami Hiramatsu
2022-05-30 12:38                                           ` Jiri Olsa
2022-05-31  1:00                                             ` Masami Hiramatsu
2022-05-04 12:43               ` Mark Rutland
2022-05-05  2:57             ` Wangshaobo (bobo)
2022-05-25 12:27               ` Mark Rutland
2022-04-27  8:54       ` Wangshaobo (bobo)
2022-03-16 10:01 ` [RFC PATCH -next v2 4/4] arm64/ftrace: implement long jump for dynamic trampolines Wang ShaoBo
2022-04-21 13:47   ` Mark Rutland
2022-03-16 14:29 ` [RFC PATCH -next v2 0/4] arm64/ftrace: support dynamic trampoline Steven Rostedt
2022-04-20 18:11 ` Steven Rostedt
2022-04-21  1:13   ` Wangshaobo (bobo)
2022-04-21 12:37     ` Steven Rostedt
2022-05-25 12:45       ` Mark Rutland
2022-05-25 13:58         ` Steven Rostedt
2022-05-25 17:26           ` Mark Rutland
2022-04-21 12:53 ` Mark Rutland

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=20220512210231.f9178a98f20a37981b1e89e3@kernel.org \
    --to=mhiramat@kernel.org \
    --cc=bobo.shaobowang@huawei.com \
    --cc=catalin.marinas@arm.com \
    --cc=cj.chengjian@huawei.com \
    --cc=huawei.libin@huawei.com \
    --cc=jolsa@kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=liwei391@huawei.com \
    --cc=mark.rutland@arm.com \
    --cc=rostedt@goodmis.org \
    --cc=will@kernel.org \
    --cc=xiexiuqi@huawei.com \
    --cc=zengshun.wu@outlook.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).