Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Sat, 25 Jul 2020 17:09:14 +0200
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
Message-ID: <20200725150914.GA3362@ubuntu>
References: <20200719155033.24201-1-oscar.carter@gmx.com>
 <20200719155033.24201-3-oscar.carter@gmx.com>
 <20200721140545.445f0258@oasis.local.home>
 <20200724161921.GA3123@ubuntu>
 <20200724123528.36ea9c9e@oasis.local.home>
 <20200724171418.GB3123@ubuntu>
 <20200724133656.76c75629@oasis.local.home>
 <20200724134020.3160dc7c@oasis.local.home>
 <20200724175500.GD3123@ubuntu>
 <20200724143457.27755412@oasis.local.home>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20200724143457.27755412@oasis.local.home>
User-Agent: Mutt/1.10.1 (2018-07-13)
Content-Transfer-Encoding: quoted-printable
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Hi Steven,

On Fri, Jul 24, 2020 at 02:34:57PM -0400, Steven Rostedt wrote:
> On Fri, 24 Jul 2020 19:55:00 +0200
> Oscar Carter <oscar.carter@gmx.com> wrote:
>
> > > Which one of the above is this patch set for?
> >
> > This patch is the result of a warning obtained with the following:
> >
> > make allmodconfig ARCH=3Dpowerpc
> > make ARCH=3Dpowerpc CROSS_COMPILE=3Dpowerpc-linux-gnu- -j4
> >
> > And with the -Wcast-function-type enabled in the top level makefile.
>
> Looking into powerpc I found this:
>
> arch/powerpc/include/asm/ftrace.h:
>
>   #ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
>   #define ARCH_SUPPORTS_FTRACE_OPS 1
>   #endif
>
>
> arch/powerpc/Kconfig:
>
> 	select HAVE_DYNAMIC_FTRACE_WITH_REGS	if MPROFILE_KERNEL
> [..]
>
>   config MPROFILE_KERNEL
> 	depends on PPC64 && CPU_LITTLE_ENDIAN && FUNCTION_TRACER
> 	def_bool $(success,$(srctree)/arch/powerpc/tools/gcc-check-mprofile-ker=
nel.sh $(CC) -I$(srctree)/include -D__KERNEL__)
>
> So, it looks like you need to be 64bit PowerPC, Little Endian, and gcc
> needs to support -mprofile.
>
> Otherwise, it falls back to the old way that does the type casting.
>
> If you are really concerned about this, I would recommend adding
> support to the architecture you care about, and then this will no
> longer be an issue.
>
> The funny part is, you can still add support for ftrace_ops, without
> adding support for DYNAMIC_FTRACE_WITH_REGS, if you only care about not
> having to do that typecast.

I agree with you. I will try to add the support for ftrace_ops without
adding support for DYNAMIC_FTRACE_WITH_REGS to the powerpc architecture.

It's a great solution to allow a clean CFI build and so, protect this
arch (powerpc) against attacks that try to modify the normal control
flow.

I take a look at the kernel documentation about port ftrace to other
architectures [1] but it is out of date.

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.

> My NAK still stands. I wont let an intrusive patch be added to the
> ftrace core code to deal with an unsupported feature in an architecture.

Don't worry. I agree with you and thanks for finding a better way to
accomplish the initial purpose.

> I would be will to add that linker trick to remove the warning. Or we
> just use that warning as incentive to get architecture developers to
> implement this feature ;-)

In my opinion it would be better to leave the code as it an make the warni=
ng
visible until this feature has been added.

> -- Steve

[1] https://www.kernel.org/doc/html/latest/trace/ftrace-design.html

Thanks again,
Oscar Carter