lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Wed, 26 Oct 2022 00:30:32 +0900 From: Masami Hiramatsu (Google) <mhiramat@...nel.org> To: Florent Revest <revest@...omium.org> Cc: Mark Rutland <mark.rutland@....com>, Masami Hiramatsu <mhiramat@...nel.org>, linux-kernel@...r.kernel.org, catalin.marinas@....com, linux-arm-kernel@...ts.infradead.org, rostedt@...dmis.org, will@...nel.org Subject: Re: [PATCH 3/4] ftrace: abstract DYNAMIC_FTRACE_WITH_ARGS accesses On Tue, 25 Oct 2022 15:20:57 +0200 Florent Revest <revest@...omium.org> wrote: > On Tue, Oct 25, 2022 at 12:30 PM Mark Rutland <mark.rutland@....com> wrote: > > > > On Tue, Oct 25, 2022 at 05:40:01PM +0900, Masami Hiramatsu wrote: > > > Hi Mark, > > > > > > On Mon, 24 Oct 2022 15:08:45 +0100 > > > Mark Rutland <mark.rutland@....com> wrote: > > > > > > > In subsequent patches we'll arrange for architectures to have an > > > > ftrace_regs which is entirely distinct from pt_regs. In preparation for > > > > this, we need to minimize the use of pt_regs to where strictly necessary > > > > in the core ftrace code. > > > > > > > > This patch adds new ftrace_regs_{get,set}_*() helpers which can be used > > > > to manipulate ftrace_regs. When CONFIG_HAVE_DYNAMIC_FTRACE_WITH_ARGS=y, > > > > these can always be used on any ftrace_regs, and when > > > > CONFIG_HAVE_DYNAMIC_FTRACE_WITH_ARGS=n these can be used when regs are > > > > available. A new ftrace_regs_has_args(fregs) helper is added which code > > > > can use to check when these are usable. > > > > > > Can you also add the ftrace_regs_query_register_offset() as a wrapper of > > > regs_query_register_offset()? I would like to use it for fprobe_events. > > > > Sure! > > > > Just to check, with FTRACE_WITH_REGS, does fprobe always sample the full > > pt_regs, or do callers also need to check ftrace_regs_has_args(fregs)? No, if the register is NOT saved, just return -ENOENT. > > Currently, we have: > config FPROBE > depends on DYNAMIC_FTRACE_WITH_REGS > > Because fprobe registers its ftrace ops with FTRACE_OPS_FL_SAVE_REGS > and calls ftrace_get_regs() to give pt_regs to registered fprobe > callbacks. > > We'll need to refactor fprobe a bit to support || > DYNAMIC_FTRACE_WITH_ARGS and therefore work on arm64. Yeah, that is what I will do. > > I ask because if neither of those are the case, with FTRACE_WITH_REGS, > > ftrace_regs_query_register_offset() would accept names of registers which might > > not have been sampled, and could give offsets to uninitialized memory. > > Indeed, if one were to call the regs_query_register_offset() on a > pt_regs that comes out of a ftrace trampoline with FTRACE_WITH_REGS, > for example in a fprobe callback, one could get offsets to > uninitialized memory already (for the registers we can't get outside > of an exception handler on arm64 for example iiuc) > > And it'd get way worse with FTRACE_WITH_ARGS if we implement > ftrace_regs_query_register_offset() by calling > regs_query_register_offset() for ftrace_regs that contain sparse > pt_regs. Ah, I got what you meant. If you consider that case, it can return -ENOTSUPPORT for FTRACE_WITH_ARGS case at this moment. I'll fill it afterwards. > > > Atop that, I'm not exactly sure what to implement for powerpc/s390/x86 here. If > > those might be used without a full pt_regs, I think > > ftrace_regs_query_register_offset() should also take the fregs as a parameter > > and use that to check which registers are available. > > I think it would make sense for a ftrace_regs_query_register_offset() > to only return offsets to the registers that are actually saved by the > arch in a ftrace_regs (whether that's WITH_ARGS or WITH_REGS). > > But that also means that if we introduce "fprobe_events" in the > tracing sysfs interface, we can't have it support a %REG syntax > compatible with the one in "kprobe_events" anyway. > > Masami, how about having "fprobe_events" only support $argN, $retval > etc but no %REG, from the beginning ? Then it would be clear to users > that fprobe can not guarantee registers and we'd never have to fake > registers when we don't have them. Users would have to make a decision > between using fprobe which is fast but only has arguments and return > value and kprobe which is slow but has all registers. Hmm, for the first implementation, it is OK. But later I need to implement the register access, because, debuginfo maps it differently. I would like to arrow perf-probe to set it up eventually. > I realize this has consequences for the kretprobe and rethook > unification plan but if we have fprobe_events support %REG at the > beginning, we'd have to break it at some point down the line anyway > right ? No, because %REG support doesn't mean we guarantee to access all registers. We can just use %REG as alias of $argN :) Thank you, > > > ... does that make sense to you? > > > > Thanks, > > Mark. -- Masami Hiramatsu (Google) <mhiramat@...nel.org>
Powered by blists - more mailing lists