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 <[email protected]>
Cc: Kees Cook <[email protected]>
Cc: Geert Uytterhoeven <[email protected]>
Cc: Nick Desaulniers <[email protected]>
Signed-off-by: Ard Biesheuvel <[email protected]>
---
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
+ Fangrui
On Tue, Oct 27, 2020 at 8:11 AM Ard Biesheuvel <[email protected]> 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 <[email protected]>
> Cc: Kees Cook <[email protected]>
> Cc: Geert Uytterhoeven <[email protected]>
> Cc: Nick Desaulniers <[email protected]>
> Signed-off-by: Ard Biesheuvel <[email protected]>
> ---
> 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
One nit about ".got" in the message:
Reviewed-by: Fangrui Song <[email protected]>
On 2020-10-27, Nick Desaulniers wrote:
>+ Fangrui
>
>On Tue, Oct 27, 2020 at 8:11 AM Ard Biesheuvel <[email protected]> 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,
.got -> .got.plt
be2881824ae9eb92 does not ASSERT on .got (it can).
Strangely *(.got) is placed in .text in arch/arm64/kernel/vmlinux.lds.S
I think that line can be removed. On x86, aarch64 and many other archs,
the start of .got.plt is the GOT base. .got is not needed (ppc/arm/riscv
use .got instead of .got.plt as the GOT base anchor).
>> 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).
Confirmed.
>> 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.
Yes, the R_AARCH64_CALL26 relocation referencing an undefined weak
symbol causes one .plt entry and one .got.plt 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 <[email protected]>
>> Cc: Kees Cook <[email protected]>
>> Cc: Geert Uytterhoeven <[email protected]>
>> Cc: Nick Desaulniers <[email protected]>
>> Signed-off-by: Ard Biesheuvel <[email protected]>
>> ---
>> 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
+++ Will Deacon [28/10/20 13:24 +0000]:
>On Wed, Oct 28, 2020 at 01:27:01PM +0100, Ard Biesheuvel wrote:
>> On Wed, 28 Oct 2020 at 11:00, Will Deacon <[email protected]> wrote:
>> > On Tue, Oct 27, 2020 at 04:11:32PM +0100, Ard Biesheuvel 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 <[email protected]>
>> > > Cc: Kees Cook <[email protected]>
>> > > Cc: Geert Uytterhoeven <[email protected]>
>> > > Cc: Nick Desaulniers <[email protected]>
>> > > Signed-off-by: Ard Biesheuvel <[email protected]>
>> > > ---
>> > > include/linux/module.h | 2 +-
>> > > 1 file changed, 1 insertion(+), 1 deletion(-)
>> >
>> > Cheers. I gave this a spin, but I unfortunately still see the following
>> > linker warning with allnoconfig:
>> >
>> > aarch64-linux-gnu-ld: warning: orphan section `.igot.plt' from `arch/arm64/kernel/head.o' being placed in section `.igot.plt'
>> >
>> > which looks unrelated to symbol_get(), but maybe it's worth knocking these
>> > things on the head (no pun intended) at the same time?
>> >
>>
>> Yeah, that is just one of those spurious sections that turns up empty
>> anyway. The head.o is a red herring, it is simply the first file
>> appearing in the link.
>>
>> This should fix it
>>
>> diff --git a/arch/arm64/kernel/vmlinux.lds.S b/arch/arm64/kernel/vmlinux.lds.S
>> index 6567d80dd15f..48b222f1c700 100644
>> --- a/arch/arm64/kernel/vmlinux.lds.S
>> +++ b/arch/arm64/kernel/vmlinux.lds.S
>> @@ -278,7 +278,7 @@ SECTIONS
>> * explicitly check instead of blindly discarding.
>> */
>> .plt : {
>> - *(.plt) *(.plt.*) *(.iplt) *(.igot)
>> + *(.plt) *(.plt.*) *(.iplt) *(.igot .igot.plt)
>> }
>> ASSERT(SIZEOF(.plt) == 0, "Unexpected run-time procedure
>> linkages detected!")
>
>Cheers, that fixes the extra warning for me. If you could send a proper
>patch, I'm happy to queue as an arm64 fix! (I'm assuming the former is going
>via Jessica, but I can also take that with her Ack).
Hi! Yes, please feel free to take this patch along with the other fix:
Acked-by: Jessica Yu <[email protected]>
Thanks,
Jessica
On Wed, 28 Oct 2020 at 11:00, Will Deacon <[email protected]> wrote:
>
> Hi Ard,
>
> On Tue, Oct 27, 2020 at 04:11:32PM +0100, Ard Biesheuvel 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 <[email protected]>
> > Cc: Kees Cook <[email protected]>
> > Cc: Geert Uytterhoeven <[email protected]>
> > Cc: Nick Desaulniers <[email protected]>
> > Signed-off-by: Ard Biesheuvel <[email protected]>
> > ---
> > include/linux/module.h | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
>
> Cheers. I gave this a spin, but I unfortunately still see the following
> linker warning with allnoconfig:
>
> aarch64-linux-gnu-ld: warning: orphan section `.igot.plt' from `arch/arm64/kernel/head.o' being placed in section `.igot.plt'
>
> which looks unrelated to symbol_get(), but maybe it's worth knocking these
> things on the head (no pun intended) at the same time?
>
Yeah, that is just one of those spurious sections that turns up empty
anyway. The head.o is a red herring, it is simply the first file
appearing in the link.
This should fix it
diff --git a/arch/arm64/kernel/vmlinux.lds.S b/arch/arm64/kernel/vmlinux.lds.S
index 6567d80dd15f..48b222f1c700 100644
--- a/arch/arm64/kernel/vmlinux.lds.S
+++ b/arch/arm64/kernel/vmlinux.lds.S
@@ -278,7 +278,7 @@ SECTIONS
* explicitly check instead of blindly discarding.
*/
.plt : {
- *(.plt) *(.plt.*) *(.iplt) *(.igot)
+ *(.plt) *(.plt.*) *(.iplt) *(.igot .igot.plt)
}
ASSERT(SIZEOF(.plt) == 0, "Unexpected run-time procedure
linkages detected!")
On Tue, 27 Oct 2020 16:11:32 +0100, Ard Biesheuvel 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).
>
> [...]
Applied to arm64 (for-next/fixes), thanks!
[1/1] module: use hidden visibility for weak symbol references
https://git.kernel.org/arm64/c/13150bc5416f
Cheers,
--
Will
https://fixes.arm64.dev
https://next.arm64.dev
https://will.arm64.dev
On Wed, Oct 28, 2020 at 03:03:44PM +0100, Jessica Yu wrote:
> +++ Will Deacon [28/10/20 13:24 +0000]:
> > On Wed, Oct 28, 2020 at 01:27:01PM +0100, Ard Biesheuvel wrote:
> > > On Wed, 28 Oct 2020 at 11:00, Will Deacon <[email protected]> wrote:
> > > > On Tue, Oct 27, 2020 at 04:11:32PM +0100, Ard Biesheuvel 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 <[email protected]>
> > > > > Cc: Kees Cook <[email protected]>
> > > > > Cc: Geert Uytterhoeven <[email protected]>
> > > > > Cc: Nick Desaulniers <[email protected]>
> > > > > Signed-off-by: Ard Biesheuvel <[email protected]>
> > > > > ---
> > > > > include/linux/module.h | 2 +-
> > > > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > > >
> > > > Cheers. I gave this a spin, but I unfortunately still see the following
> > > > linker warning with allnoconfig:
> > > >
> > > > aarch64-linux-gnu-ld: warning: orphan section `.igot.plt' from `arch/arm64/kernel/head.o' being placed in section `.igot.plt'
> > > >
> > > > which looks unrelated to symbol_get(), but maybe it's worth knocking these
> > > > things on the head (no pun intended) at the same time?
> > > >
> > >
> > > Yeah, that is just one of those spurious sections that turns up empty
> > > anyway. The head.o is a red herring, it is simply the first file
> > > appearing in the link.
> > >
> > > This should fix it
> > >
> > > diff --git a/arch/arm64/kernel/vmlinux.lds.S b/arch/arm64/kernel/vmlinux.lds.S
> > > index 6567d80dd15f..48b222f1c700 100644
> > > --- a/arch/arm64/kernel/vmlinux.lds.S
> > > +++ b/arch/arm64/kernel/vmlinux.lds.S
> > > @@ -278,7 +278,7 @@ SECTIONS
> > > * explicitly check instead of blindly discarding.
> > > */
> > > .plt : {
> > > - *(.plt) *(.plt.*) *(.iplt) *(.igot)
> > > + *(.plt) *(.plt.*) *(.iplt) *(.igot .igot.plt)
> > > }
> > > ASSERT(SIZEOF(.plt) == 0, "Unexpected run-time procedure
> > > linkages detected!")
> >
> > Cheers, that fixes the extra warning for me. If you could send a proper
> > patch, I'm happy to queue as an arm64 fix! (I'm assuming the former is going
> > via Jessica, but I can also take that with her Ack).
>
> Hi! Yes, please feel free to take this patch along with the other fix:
>
> Acked-by: Jessica Yu <[email protected]>
Cheers, Jessica -- I'll queue them in a sec!
Will
On Wed, Oct 28, 2020 at 01:27:01PM +0100, Ard Biesheuvel wrote:
> On Wed, 28 Oct 2020 at 11:00, Will Deacon <[email protected]> wrote:
> > On Tue, Oct 27, 2020 at 04:11:32PM +0100, Ard Biesheuvel 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 <[email protected]>
> > > Cc: Kees Cook <[email protected]>
> > > Cc: Geert Uytterhoeven <[email protected]>
> > > Cc: Nick Desaulniers <[email protected]>
> > > Signed-off-by: Ard Biesheuvel <[email protected]>
> > > ---
> > > include/linux/module.h | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > Cheers. I gave this a spin, but I unfortunately still see the following
> > linker warning with allnoconfig:
> >
> > aarch64-linux-gnu-ld: warning: orphan section `.igot.plt' from `arch/arm64/kernel/head.o' being placed in section `.igot.plt'
> >
> > which looks unrelated to symbol_get(), but maybe it's worth knocking these
> > things on the head (no pun intended) at the same time?
> >
>
> Yeah, that is just one of those spurious sections that turns up empty
> anyway. The head.o is a red herring, it is simply the first file
> appearing in the link.
>
> This should fix it
>
> diff --git a/arch/arm64/kernel/vmlinux.lds.S b/arch/arm64/kernel/vmlinux.lds.S
> index 6567d80dd15f..48b222f1c700 100644
> --- a/arch/arm64/kernel/vmlinux.lds.S
> +++ b/arch/arm64/kernel/vmlinux.lds.S
> @@ -278,7 +278,7 @@ SECTIONS
> * explicitly check instead of blindly discarding.
> */
> .plt : {
> - *(.plt) *(.plt.*) *(.iplt) *(.igot)
> + *(.plt) *(.plt.*) *(.iplt) *(.igot .igot.plt)
> }
> ASSERT(SIZEOF(.plt) == 0, "Unexpected run-time procedure
> linkages detected!")
Cheers, that fixes the extra warning for me. If you could send a proper
patch, I'm happy to queue as an arm64 fix! (I'm assuming the former is going
via Jessica, but I can also take that with her Ack).
Will
Hi Ard,
On Tue, Oct 27, 2020 at 04:11:32PM +0100, Ard Biesheuvel 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 <[email protected]>
> Cc: Kees Cook <[email protected]>
> Cc: Geert Uytterhoeven <[email protected]>
> Cc: Nick Desaulniers <[email protected]>
> Signed-off-by: Ard Biesheuvel <[email protected]>
> ---
> include/linux/module.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
Cheers. I gave this a spin, but I unfortunately still see the following
linker warning with allnoconfig:
aarch64-linux-gnu-ld: warning: orphan section `.igot.plt' from `arch/arm64/kernel/head.o' being placed in section `.igot.plt'
which looks unrelated to symbol_get(), but maybe it's worth knocking these
things on the head (no pun intended) at the same time?
Cheers,
Will