Kernel-hardening archive on lore.kernel.org
 help / color / Atom feed
From: Oscar Carter <oscar.carter@gmx.com>
To: Steven Rostedt <rostedt@goodmis.org>
Cc: Oscar Carter <oscar.carter@gmx.com>,
	Ingo Molnar <mingo@redhat.com>, Kees Cook <keescook@chromium.org>,
	linux-kernel@vger.kernel.org,
	kernel-hardening@lists.openwall.com, Jann Horn <jannh@google.com>
Subject: Re: [PATCH v2 2/2] kernel/trace: Remove function callback casts
Date: Fri, 31 Jul 2020 16:41:32 +0200
Message-ID: <20200731144132.GA6515@ubuntu> (raw)
In-Reply-To: <20200727095306.7f369949@oasis.local.home>

Hi Steven,

On Mon, Jul 27, 2020 at 09:53:06AM -0400, Steven Rostedt wrote:
> On Sun, 26 Jul 2020 17:52:42 +0200
> Oscar Carter <oscar.carter@gmx.com> wrote:
>
> > > If I try to do this I will need some help. Some info that point me to the
> > > right direction would be greatly appreciated. Some advice about what
> > > functions I will need to implement would be really helpfull. Or point me
> > > to the right piece of code that I can pick as base point.
> >
> > I've been searching and reading the code as much as possible. I've found
> > two patches that I think can be useful to guide me. One [1] adds support
> > for ftrace_ops to the riscv architecture. The other one [2] adds support
> > for ftrace_ops to the parisc architecture.
> >
> > [1] commit 71e736a7d655 ("riscv/ftrace: Add ARCH_SUPPORTS_FTRACE_OPS support")
> > [2] commit d562aca37a54 ("parisc/ftrace: Add ARCH_SUPPORTS_FTRACE_OPS support")
> >
> > Due to powerpc arch calls the needed functions from assembly, I based my
> > idea on the patch for the RISCV arch.
> >
> > Can something like the following work?
> >
> > diff --git a/arch/powerpc/include/asm/ftrace.h b/arch/powerpc/include/asm/ftrace.h
> > index bc76970b6ee5..1c51ff5afae1 100644
> > --- a/arch/powerpc/include/asm/ftrace.h
> > +++ b/arch/powerpc/include/asm/ftrace.h
> > @@ -61,9 +61,8 @@ struct dyn_arch_ftrace {
> >  };
> >  #endif /* __ASSEMBLY__ */
> >
> > -#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
> >  #define ARCH_SUPPORTS_FTRACE_OPS 1
> > -#endif
> > +
> >  #endif /* CONFIG_FUNCTION_TRACER */
> >
> >  #ifndef __ASSEMBLY__
> > diff --git a/arch/powerpc/kernel/trace/ftrace_32.S b/arch/powerpc/kernel/trace/ftrace_32.S
> > index e023ae59c429..e69a4e945986 100644
> > --- a/arch/powerpc/kernel/trace/ftrace_32.S
> > +++ b/arch/powerpc/kernel/trace/ftrace_32.S
> > @@ -29,6 +29,10 @@ _GLOBAL(ftrace_caller)
> >         MCOUNT_SAVE_FRAME
> >         /* r3 ends up with link register */
> >         subi    r3, r3, MCOUNT_INSN_SIZE
> > +
> > +       /* Set ftrace_ops (r5) to the global variable function_trace_op */
> > +       /* Set pt_regs (r6) to NULL */
> > +
> >  .globl ftrace_call
> >  ftrace_call:
> >         bl      ftrace_stub
> > diff --git a/arch/powerpc/kernel/trace/ftrace_64_pg.S b/arch/powerpc/kernel/trace/ftrace_64_pg.S
> > index 6708e24db0ab..a741448b1df9 100644
> > --- a/arch/powerpc/kernel/trace/ftrace_64_pg.S
> > +++ b/arch/powerpc/kernel/trace/ftrace_64_pg.S
> > @@ -22,6 +22,10 @@ _GLOBAL_TOC(ftrace_caller)
> >         std     r3, 128(r1)
> >         ld      r4, 16(r11)
> >         subi    r3, r3, MCOUNT_INSN_SIZE
> > +
> > +       /* Set ftrace_ops (r5) to the global variable function_trace_op */
> > +       /* Set pt_regs (r6) to NULL */
>
> I'm guessing you are going to do the above here. If so, this looks correct.
>
> > +
> >  .globl ftrace_call
> >  ftrace_call:
> >         bl      ftrace_stub
> >
> > To add support for ftrace_ops to the powerpc architecture is only necessary
> > to fill the r5 and r6 registers before the call to ftrace_stub in all the
> > cases. The register r5 is a pointer to ftrace_ops struct and the register
> > r6 is a pointer to pt_regs struct. These registers are the third and fourth
> > parameters of a function with the following prototype. The first and second
> > ones are yet set.
>
> I guess you mean that the first and second ones are already set. But,
> yeah, you are on the correct path here!
>
> >
> > void func(unsigned long ip, unsigned long parent_ip,
> > 	  struct ftrace_ops *ops, struct pt_regs *regs);
> >
> > Am I in the right direction? or am I totally wrong?
>
> No, you don't look wrong. But the true test is to try it out :-)

Of course. I will do the commented work, I will test it and then I will send
a new patch.

> Don't forget to update ftrace_32.S as well.
>
>
> >
> > Thanks for your time and patience.
>
> My pleasure. Thanks for doing this. The more people that understand all
> this, the better!

Thanks for your guidance and advices.

> -- Steve

Regards,
Oscar Carter

      reply index

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-19 15:50 [PATCH v2 0/2] " Oscar Carter
2020-07-19 15:50 ` [PATCH v2 1/2] kernel/trace: Prepare to remove " Oscar Carter
2020-07-19 15:50 ` [PATCH v2 2/2] kernel/trace: Remove " Oscar Carter
2020-07-21 18:05   ` Steven Rostedt
2020-07-24 16:19     ` Oscar Carter
2020-07-24 16:35       ` Steven Rostedt
2020-07-24 17:14         ` Oscar Carter
2020-07-24 17:24           ` Oscar Carter
2020-07-24 17:36           ` Steven Rostedt
2020-07-24 17:40             ` Steven Rostedt
2020-07-24 17:48               ` Steven Rostedt
2020-07-24 17:55               ` Oscar Carter
2020-07-24 18:34                 ` Steven Rostedt
2020-07-25 15:09                   ` Oscar Carter
2020-07-25 15:19                     ` Oscar Carter
2020-07-26 15:52                     ` Oscar Carter
2020-07-27 13:53                       ` Steven Rostedt
2020-07-31 14:41                         ` Oscar Carter [this message]

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=20200731144132.GA6515@ubuntu \
    --to=oscar.carter@gmx.com \
    --cc=jannh@google.com \
    --cc=keescook@chromium.org \
    --cc=kernel-hardening@lists.openwall.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=rostedt@goodmis.org \
    /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

Kernel-hardening archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/kernel-hardening/0 kernel-hardening/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 kernel-hardening kernel-hardening/ https://lore.kernel.org/kernel-hardening \
		kernel-hardening@lists.openwall.com
	public-inbox-index kernel-hardening

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/com.openwall.lists.kernel-hardening


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git