From: Nick Desaulniers <ndesaulniers@google.com>
To: Ard Biesheuvel <ardb@kernel.org>, Fangrui Song <maskray@google.com>
Cc: LKML <linux-kernel@vger.kernel.org>,
Linux ARM <linux-arm-kernel@lists.infradead.org>,
Will Deacon <will@kernel.org>,
Catalin Marinas <catalin.marinas@arm.com>,
Jessica Yu <jeyu@kernel.org>, Kees Cook <keescook@chromium.org>,
Geert Uytterhoeven <geert@linux-m68k.org>,
clang-built-linux <clang-built-linux@googlegroups.com>,
linux-toolchains@vger.kernel.org
Subject: Re: [PATCH] module: use hidden visibility for weak symbol references
Date: Tue, 27 Oct 2020 11:27:33 -0700 [thread overview]
Message-ID: <CAKwvOd=XHAGotJ38o=hZTwi89XvCyshaUtWezZQ-k6aRT20xwQ@mail.gmail.com> (raw)
In-Reply-To: <20201027151132.14066-1-ardb@kernel.org>
+ Fangrui
On Tue, Oct 27, 2020 at 8:11 AM Ard Biesheuvel <ardb@kernel.org> wrote:
>
> Geert reports that commit be2881824ae9eb92 ("arm64/build: Assert for
> unwanted sections") results in build errors on arm64 for configurations
> that have CONFIG_MODULES disabled.
>
> The commit in question added ASSERT()s to the arm64 linker script to
> ensure that linker generated sections such as .got, .plt etc are empty,
> but as it turns out, there are corner cases where the linker does emit
> content into those sections. More specifically, weak references to
> function symbols (which can remain unsatisfied, and can therefore not
> be emitted as relative references) will be emitted as GOT and PLT
> entries when linking the kernel in PIE mode (which is the case when
> CONFIG_RELOCATABLE is enabled, which is on by default).
>
> What happens is that code such as
>
> struct device *(*fn)(struct device *dev);
> struct device *iommu_device;
>
> fn = symbol_get(mdev_get_iommu_device);
> if (fn) {
> iommu_device = fn(dev);
>
> essentially gets converted into the following when CONFIG_MODULES is off:
>
> struct device *iommu_device;
>
> if (&mdev_get_iommu_device) {
> iommu_device = mdev_get_iommu_device(dev);
>
> where mdev_get_iommu_device is emitted as a weak symbol reference into
> the object file. The first reference is decorated with an ordinary
> ABS64 data relocation (which yields 0x0 if the reference remains
> unsatisfied). However, the indirect call is turned into a direct call
> covered by a R_AARCH64_CALL26 relocation, which is converted into a
> call via a PLT entry taking the target address from the associated
> GOT entry.
>
> Given that such GOT and PLT entries are unnecessary for fully linked
> binaries such as the kernel, let's give these weak symbol references
> hidden visibility, so that the linker knows that the weak reference
> via R_AARCH64_CALL26 can simply remain unsatisfied.
>
> Cc: Jessica Yu <jeyu@kernel.org>
> Cc: Kees Cook <keescook@chromium.org>
> Cc: Geert Uytterhoeven <geert@linux-m68k.org>
> Cc: Nick Desaulniers <ndesaulniers@google.com>
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> ---
> include/linux/module.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/linux/module.h b/include/linux/module.h
> index 7ccdf87f376f..6264617bab4d 100644
> --- a/include/linux/module.h
> +++ b/include/linux/module.h
> @@ -740,7 +740,7 @@ static inline bool within_module(unsigned long addr, const struct module *mod)
> }
>
> /* Get/put a kernel symbol (calls should be symmetric) */
> -#define symbol_get(x) ({ extern typeof(x) x __attribute__((weak)); &(x); })
> +#define symbol_get(x) ({ extern typeof(x) x __attribute__((weak,visibility("hidden"))); &(x); })
> #define symbol_put(x) do { } while (0)
> #define symbol_put_addr(x) do { } while (0)
>
> --
> 2.17.1
>
--
Thanks,
~Nick Desaulniers
next parent reply other threads:[~2020-10-27 18:27 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <20201027151132.14066-1-ardb@kernel.org>
2020-10-27 18:27 ` Nick Desaulniers [this message]
2020-10-27 22:18 ` [PATCH] module: use hidden visibility for weak symbol references Fangrui Song
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='CAKwvOd=XHAGotJ38o=hZTwi89XvCyshaUtWezZQ-k6aRT20xwQ@mail.gmail.com' \
--to=ndesaulniers@google.com \
--cc=ardb@kernel.org \
--cc=catalin.marinas@arm.com \
--cc=clang-built-linux@googlegroups.com \
--cc=geert@linux-m68k.org \
--cc=jeyu@kernel.org \
--cc=keescook@chromium.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-toolchains@vger.kernel.org \
--cc=maskray@google.com \
--cc=will@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).