All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mark Rutland <mark.rutland@arm.com>
To: Ard Biesheuvel <ardb@kernel.org>
Cc: Yinan Liu <yinan@linux.alibaba.com>,
	Steven Rostedt <rostedt@goodmis.org>,
	"open list:LINUX FOR POWERPC (32-BIT AND 64-BIT)" 
	<linuxppc-dev@lists.ozlabs.org>,
	Sachin Sant <sachinp@linux.ibm.com>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Kees Cook <keescook@chromium.org>
Subject: Re: [powerpc] ftrace warning kernel/trace/ftrace.c:2068 with code-patching selftests
Date: Thu, 27 Jan 2022 14:54:47 +0000	[thread overview]
Message-ID: <YfKyNwYl/pkmVmDm@FVFF77S0Q05N> (raw)
In-Reply-To: <CAMj1kXHnQcHmxRrTBQmj0Z2JJ6iWvNCQqSjvPqG_oedWpikfSA@mail.gmail.com>

On Thu, Jan 27, 2022 at 02:59:31PM +0100, Ard Biesheuvel wrote:
> On Thu, 27 Jan 2022 at 14:24, Mark Rutland <mark.rutland@arm.com> wrote:
> >
> > On Thu, Jan 27, 2022 at 02:07:03PM +0100, Ard Biesheuvel wrote:
> > > I suppose that on arm64, we can work around this by passing
> > > --apply-dynamic-relocs to the linker, so that all R_AARCH64_RELATIVE
> > > targets are prepopulated with the link time value of the respective
> > > addresses. It does cause some bloat, which is why we disable that
> > > today, but we could make that dependent on ftrace being enabled.
> >
> > We'd also need to teach the build-time sort to update the relocations, unless
> > you mean to also change the boot-time reloc code to RMW with the offset?
> 
> Why would that be necessary? Every RELA entry has the same effect on
> its target address, as it just adds a fixed offset.

Currently in relocate_kernel() we generate the absolute address from the
relocation alone, with the core of the relocation logic being as follows, with
x9 being the pointer to a RELA entry, and x23 being the offset relative to the
default load address:

	ldp     x12, x13, [x9], #24
	ldr	x14, [x9, #-8]

	add     x14, x14, x23                   // relocate
	str     x14, [x12, x23]

... and (as per another reply), a sample RELA entry currently contains:

	0xffff8000090b1ab0	// default load VA of pointer to update
	0x0000000000000403	// R_AARCH64_RELATIVE
	0xffff8000090b6000	// default load VA of addr to write

So either:

* That code stays as-is, and we must update the relocs to correspond to their
  new sorted locations, or we'll blat the sorted values with the original
  relocs as we do today.

* The code needs to change to RMW: read the existing value, add the offset
  (ignoring the content of the RELA entry's addend field), and write it back.
  This is what I meant when I said "change the boot-time reloc code to RMW with
  the offset".

Does that make sense, or have I misunderstood?

Thanks,
Mark.

WARNING: multiple messages have this Message-ID (diff)
From: Mark Rutland <mark.rutland@arm.com>
To: Ard Biesheuvel <ardb@kernel.org>
Cc: Kees Cook <keescook@chromium.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Steven Rostedt <rostedt@goodmis.org>,
	Sachin Sant <sachinp@linux.ibm.com>,
	Yinan Liu <yinan@linux.alibaba.com>,
	"open list:LINUX FOR POWERPC \(32-BIT AND 64-BIT\)"
	<linuxppc-dev@lists.ozlabs.org>
Subject: Re: [powerpc] ftrace warning kernel/trace/ftrace.c:2068 with code-patching selftests
Date: Thu, 27 Jan 2022 14:54:47 +0000	[thread overview]
Message-ID: <YfKyNwYl/pkmVmDm@FVFF77S0Q05N> (raw)
In-Reply-To: <CAMj1kXHnQcHmxRrTBQmj0Z2JJ6iWvNCQqSjvPqG_oedWpikfSA@mail.gmail.com>

On Thu, Jan 27, 2022 at 02:59:31PM +0100, Ard Biesheuvel wrote:
> On Thu, 27 Jan 2022 at 14:24, Mark Rutland <mark.rutland@arm.com> wrote:
> >
> > On Thu, Jan 27, 2022 at 02:07:03PM +0100, Ard Biesheuvel wrote:
> > > I suppose that on arm64, we can work around this by passing
> > > --apply-dynamic-relocs to the linker, so that all R_AARCH64_RELATIVE
> > > targets are prepopulated with the link time value of the respective
> > > addresses. It does cause some bloat, which is why we disable that
> > > today, but we could make that dependent on ftrace being enabled.
> >
> > We'd also need to teach the build-time sort to update the relocations, unless
> > you mean to also change the boot-time reloc code to RMW with the offset?
> 
> Why would that be necessary? Every RELA entry has the same effect on
> its target address, as it just adds a fixed offset.

Currently in relocate_kernel() we generate the absolute address from the
relocation alone, with the core of the relocation logic being as follows, with
x9 being the pointer to a RELA entry, and x23 being the offset relative to the
default load address:

	ldp     x12, x13, [x9], #24
	ldr	x14, [x9, #-8]

	add     x14, x14, x23                   // relocate
	str     x14, [x12, x23]

... and (as per another reply), a sample RELA entry currently contains:

	0xffff8000090b1ab0	// default load VA of pointer to update
	0x0000000000000403	// R_AARCH64_RELATIVE
	0xffff8000090b6000	// default load VA of addr to write

So either:

* That code stays as-is, and we must update the relocs to correspond to their
  new sorted locations, or we'll blat the sorted values with the original
  relocs as we do today.

* The code needs to change to RMW: read the existing value, add the offset
  (ignoring the content of the RELA entry's addend field), and write it back.
  This is what I meant when I said "change the boot-time reloc code to RMW with
  the offset".

Does that make sense, or have I misunderstood?

Thanks,
Mark.

  reply	other threads:[~2022-01-27 14:55 UTC|newest]

Thread overview: 46+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-01-24  9:19 [powerpc] ftrace warning kernel/trace/ftrace.c:2068 with code-patching selftests Sachin Sant
2022-01-24  9:19 ` Sachin Sant
2022-01-24 12:15 ` Yinan Liu
2022-01-24 16:45   ` Steven Rostedt
2022-01-25  3:20     ` Yinan Liu
2022-01-26 14:37       ` Mark Rutland
2022-01-27 11:46         ` Mark Rutland
2022-01-27 11:46           ` Mark Rutland
2022-01-27 12:03           ` Ard Biesheuvel
2022-01-27 12:03             ` Ard Biesheuvel
2022-01-27 12:20             ` Mark Rutland
2022-01-27 12:20               ` Mark Rutland
2022-01-27 12:22               ` Ard Biesheuvel
2022-01-27 12:22                 ` Ard Biesheuvel
2022-01-27 12:59                 ` Mark Rutland
2022-01-27 12:59                   ` Mark Rutland
2022-01-27 13:07                   ` Ard Biesheuvel
2022-01-27 13:07                     ` Ard Biesheuvel
2022-01-27 13:24                     ` Mark Rutland
2022-01-27 13:24                       ` Mark Rutland
2022-01-27 13:59                       ` Ard Biesheuvel
2022-01-27 13:59                         ` Ard Biesheuvel
2022-01-27 14:54                         ` Mark Rutland [this message]
2022-01-27 14:54                           ` Mark Rutland
2022-01-27 15:01                           ` Ard Biesheuvel
2022-01-27 15:01                             ` Ard Biesheuvel
2022-01-27 12:04           ` Sven Schnelle
2022-01-27 12:04             ` Sven Schnelle
2022-01-27 12:27             ` Mark Rutland
2022-01-27 12:27               ` Mark Rutland
2022-01-27 12:46               ` Steven Rostedt
2022-01-27 12:46                 ` Steven Rostedt
2022-01-27 13:08                 ` Mark Rutland
2022-01-27 13:08                   ` Mark Rutland
2022-01-27 13:16                   ` Sven Schnelle
2022-01-27 13:16                     ` Sven Schnelle
2022-01-27 13:33                     ` Mark Rutland
2022-01-27 13:33                       ` Mark Rutland
2022-01-27 13:55                       ` Steven Rostedt
2022-01-27 13:55                         ` Steven Rostedt
2022-01-27 14:56                         ` Mark Rutland
2022-01-27 14:56                           ` Mark Rutland
2022-01-27 16:41           ` Kees Cook
2022-01-27 16:41             ` Kees Cook
2022-01-25  4:00     ` Sachin Sant
2022-01-25 14:28       ` Steven Rostedt

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=YfKyNwYl/pkmVmDm@FVFF77S0Q05N \
    --to=mark.rutland@arm.com \
    --cc=ardb@kernel.org \
    --cc=keescook@chromium.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=rostedt@goodmis.org \
    --cc=sachinp@linux.ibm.com \
    --cc=yinan@linux.alibaba.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.