All of lore.kernel.org
 help / color / mirror / Atom feed
From: Josh Poimboeuf <jpoimboe@redhat.com>
To: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>,
	Mark Rutland <mark.rutland@arm.com>,
	linux-arch <linux-arch@vger.kernel.org>,
	Arnd Bergmann <arnd@arndb.de>,
	guillaume.gardet@arm.com, Marc Zyngier <marc.zyngier@arm.com>,
	the arch/x86 maintainers <x86@kernel.org>,
	Will Deacon <will.deacon@arm.com>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	James Morse <james.morse@arm.com>, Jessica Yu <jeyu@kernel.org>,
	Ingo Molnar <mingo@kernel.org>,
	linux-arm-kernel <linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH] module/ksymtab: use 64-bit relative reference for target symbol
Date: Fri, 24 May 2019 11:31:04 -0500	[thread overview]
Message-ID: <20190524163104.o6xh54x4ngbihneb@treble> (raw)
In-Reply-To: <CAKv+Gu9DLf9y2uqTo407gLK3AX3pq+HGFxytsoR9C2zfGdUc-A@mail.gmail.com>

On Fri, May 24, 2019 at 05:55:37PM +0200, Ard Biesheuvel wrote:
> On Fri, 24 May 2019 at 17:21, Josh Poimboeuf <jpoimboe@redhat.com> wrote:
> >
> > On Thu, May 23, 2019 at 10:29:39AM +0100, Ard Biesheuvel wrote:
> > >
> > >
> > > On 5/23/19 10:18 AM, Will Deacon wrote:
> > > > On Thu, May 23, 2019 at 09:41:40AM +0100, Ard Biesheuvel wrote:
> > > > >
> > > > >
> > > > > On 5/22/19 5:28 PM, Ard Biesheuvel wrote:
> > > > > >
> > > > > >
> > > > > > On 5/22/19 4:02 PM, Ard Biesheuvel wrote:
> > > > > > > The following commit
> > > > > > >
> > > > > > >     7290d5809571 ("module: use relative references for __ksymtab entries")
> > > > > > >
> > > > > > > updated the ksymtab handling of some KASLR capable architectures
> > > > > > > so that ksymtab entries are emitted as pairs of 32-bit relative
> > > > > > > references. This reduces the size of the entries, but more
> > > > > > > importantly, it gets rid of statically assigned absolute
> > > > > > > addresses, which require fixing up at boot time if the kernel
> > > > > > > is self relocating (which takes a 24 byte RELA entry for each
> > > > > > > member of the ksymtab struct).
> > > > > > >
> > > > > > > Since ksymtab entries are always part of the same module as the
> > > > > > > symbol they export (or of the core kernel), it was assumed at the
> > > > > > > time that a 32-bit relative reference is always sufficient to
> > > > > > > capture the offset between a ksymtab entry and its target symbol.
> > > > > > >
> > > > > > > Unfortunately, this is not always true: in the case of per-CPU
> > > > > > > variables, a per-CPU variable's base address (which usually differs
> > > > > > > from the actual address of any of its per-CPU copies) could be at
> > > > > > > an arbitrary offset from the ksymtab entry, and so it may be out
> > > > > > > of range for a 32-bit relative reference.
> > > > > > >
> > > > >
> > > > > (Apologies for the 3-act monologue)
> > > >
> > > > Exposition, development and recapitulation ;)
> > > >
> > > > > This turns out to be incorrect. The symbol address of per-CPU variables
> > > > > exported by modules is always in the vicinity of __per_cpu_start, and so it
> > > > > is simply a matter of making sure that the core kernel is in range for
> > > > > module ksymtab entries containing 32-bit relative references.
> > > > >
> > > > > When running the arm64 with kaslr enabled, we currently randomize the module
> > > > > space based on the range of ADRP/ADD instruction pairs, which have a -/+ 4
> > > > > GB range rather than the -/+ 2 GB range of 32-bit place relative data
> > > > > relocations. So we can fix this by simply reducing the randomization window
> > > > > to 2 GB.
> > > >
> > > > Makes sense. Do you see the need for an option to disable PREL relocs
> > > > altogether in case somebody wants the additional randomization range?
> > > >
> > >
> > > No, not really. To be honest, I don't think
> > > CONFIG_RANDOMIZE_MODULE_REGION_FULL is that useful to begin with, and the
> > > only reason we enabled it by default at the time was to ensure that the PLT
> > > code got some coverage after we introduced it.
> >
> > In code, percpu variables are accessed with absolute relocations, right?
> 
> No, they are accessed just like ordinary symbols, so PC32 references
> on x86 or ADRP/ADD references on arm64 are both quite common.

Ah, right, now I see some PC32 percpu references.

So if PC32 references are sufficient for code, why aren't they
sufficient for ksymtab entries?  Isn't the ksymtab data address closer
to the percpu data than the code?  Do you have an example of an out of
range ksymtab reference?

-- 
Josh

WARNING: multiple messages have this Message-ID (diff)
From: Josh Poimboeuf <jpoimboe@redhat.com>
To: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Mark Rutland <mark.rutland@arm.com>,
	linux-arch <linux-arch@vger.kernel.org>,
	Arnd Bergmann <arnd@arndb.de>,
	guillaume.gardet@arm.com, Marc Zyngier <marc.zyngier@arm.com>,
	the arch/x86 maintainers <x86@kernel.org>,
	Will Deacon <will.deacon@arm.com>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	James Morse <james.morse@arm.com>,
	Ard Biesheuvel <ard.biesheuvel@arm.com>,
	Jessica Yu <jeyu@kernel.org>, Ingo Molnar <mingo@kernel.org>,
	linux-arm-kernel <linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH] module/ksymtab: use 64-bit relative reference for target symbol
Date: Fri, 24 May 2019 11:31:04 -0500	[thread overview]
Message-ID: <20190524163104.o6xh54x4ngbihneb@treble> (raw)
In-Reply-To: <CAKv+Gu9DLf9y2uqTo407gLK3AX3pq+HGFxytsoR9C2zfGdUc-A@mail.gmail.com>

On Fri, May 24, 2019 at 05:55:37PM +0200, Ard Biesheuvel wrote:
> On Fri, 24 May 2019 at 17:21, Josh Poimboeuf <jpoimboe@redhat.com> wrote:
> >
> > On Thu, May 23, 2019 at 10:29:39AM +0100, Ard Biesheuvel wrote:
> > >
> > >
> > > On 5/23/19 10:18 AM, Will Deacon wrote:
> > > > On Thu, May 23, 2019 at 09:41:40AM +0100, Ard Biesheuvel wrote:
> > > > >
> > > > >
> > > > > On 5/22/19 5:28 PM, Ard Biesheuvel wrote:
> > > > > >
> > > > > >
> > > > > > On 5/22/19 4:02 PM, Ard Biesheuvel wrote:
> > > > > > > The following commit
> > > > > > >
> > > > > > >     7290d5809571 ("module: use relative references for __ksymtab entries")
> > > > > > >
> > > > > > > updated the ksymtab handling of some KASLR capable architectures
> > > > > > > so that ksymtab entries are emitted as pairs of 32-bit relative
> > > > > > > references. This reduces the size of the entries, but more
> > > > > > > importantly, it gets rid of statically assigned absolute
> > > > > > > addresses, which require fixing up at boot time if the kernel
> > > > > > > is self relocating (which takes a 24 byte RELA entry for each
> > > > > > > member of the ksymtab struct).
> > > > > > >
> > > > > > > Since ksymtab entries are always part of the same module as the
> > > > > > > symbol they export (or of the core kernel), it was assumed at the
> > > > > > > time that a 32-bit relative reference is always sufficient to
> > > > > > > capture the offset between a ksymtab entry and its target symbol.
> > > > > > >
> > > > > > > Unfortunately, this is not always true: in the case of per-CPU
> > > > > > > variables, a per-CPU variable's base address (which usually differs
> > > > > > > from the actual address of any of its per-CPU copies) could be at
> > > > > > > an arbitrary offset from the ksymtab entry, and so it may be out
> > > > > > > of range for a 32-bit relative reference.
> > > > > > >
> > > > >
> > > > > (Apologies for the 3-act monologue)
> > > >
> > > > Exposition, development and recapitulation ;)
> > > >
> > > > > This turns out to be incorrect. The symbol address of per-CPU variables
> > > > > exported by modules is always in the vicinity of __per_cpu_start, and so it
> > > > > is simply a matter of making sure that the core kernel is in range for
> > > > > module ksymtab entries containing 32-bit relative references.
> > > > >
> > > > > When running the arm64 with kaslr enabled, we currently randomize the module
> > > > > space based on the range of ADRP/ADD instruction pairs, which have a -/+ 4
> > > > > GB range rather than the -/+ 2 GB range of 32-bit place relative data
> > > > > relocations. So we can fix this by simply reducing the randomization window
> > > > > to 2 GB.
> > > >
> > > > Makes sense. Do you see the need for an option to disable PREL relocs
> > > > altogether in case somebody wants the additional randomization range?
> > > >
> > >
> > > No, not really. To be honest, I don't think
> > > CONFIG_RANDOMIZE_MODULE_REGION_FULL is that useful to begin with, and the
> > > only reason we enabled it by default at the time was to ensure that the PLT
> > > code got some coverage after we introduced it.
> >
> > In code, percpu variables are accessed with absolute relocations, right?
> 
> No, they are accessed just like ordinary symbols, so PC32 references
> on x86 or ADRP/ADD references on arm64 are both quite common.

Ah, right, now I see some PC32 percpu references.

So if PC32 references are sufficient for code, why aren't they
sufficient for ksymtab entries?  Isn't the ksymtab data address closer
to the percpu data than the code?  Do you have an example of an out of
range ksymtab reference?

-- 
Josh

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

  reply	other threads:[~2019-05-24 16:31 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-05-22 15:02 [PATCH] module/ksymtab: use 64-bit relative reference for target symbol Ard Biesheuvel
2019-05-22 15:02 ` Ard Biesheuvel
2019-05-22 16:28 ` Ard Biesheuvel
2019-05-22 16:28   ` Ard Biesheuvel
2019-05-23  8:41   ` Ard Biesheuvel
2019-05-23  8:41     ` Ard Biesheuvel
2019-05-23  9:18     ` Will Deacon
2019-05-23  9:18       ` Will Deacon
2019-05-23  9:29       ` Ard Biesheuvel
2019-05-23  9:29         ` Ard Biesheuvel
2019-05-24 15:20         ` Josh Poimboeuf
2019-05-24 15:20           ` Josh Poimboeuf
2019-05-24 15:55           ` Ard Biesheuvel
2019-05-24 15:55             ` Ard Biesheuvel
2019-05-24 16:31             ` Josh Poimboeuf [this message]
2019-05-24 16:31               ` Josh Poimboeuf
2019-05-24 16:33               ` Ard Biesheuvel
2019-05-24 16:33                 ` Ard Biesheuvel
2019-05-29 13:14 ` Sasha Levin

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=20190524163104.o6xh54x4ngbihneb@treble \
    --to=jpoimboe@redhat.com \
    --cc=ard.biesheuvel@arm.com \
    --cc=ard.biesheuvel@linaro.org \
    --cc=arnd@arndb.de \
    --cc=guillaume.gardet@arm.com \
    --cc=james.morse@arm.com \
    --cc=jeyu@kernel.org \
    --cc=linux-arch@vger.kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=marc.zyngier@arm.com \
    --cc=mark.rutland@arm.com \
    --cc=mingo@kernel.org \
    --cc=will.deacon@arm.com \
    --cc=x86@kernel.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
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.