* Re: [PATCH] module: Fix link failure due to invalid relocation on namespace offset
2019-09-11 12:26 [PATCH] module: Fix link failure due to invalid relocation on namespace offset Will Deacon
@ 2019-09-11 13:35 ` Jessica Yu
2019-09-11 16:40 ` Will Deacon
2019-09-11 16:51 ` Ard Biesheuvel
2019-09-11 18:26 ` Jessica Yu
2 siblings, 1 reply; 6+ messages in thread
From: Jessica Yu @ 2019-09-11 13:35 UTC (permalink / raw)
To: Will Deacon
Cc: linux-kernel, maco, gregkh, Matthias Maennich, Ard Biesheuvel,
Catalin Marinas
+++ Will Deacon [11/09/19 13:26 +0100]:
>Commit 8651ec01daed ("module: add support for symbol namespaces.")
>broke linking for arm64 defconfig:
>
> | lib/crypto/arc4.o: In function `__ksymtab_arc4_setkey':
> | arc4.c:(___ksymtab+arc4_setkey+0x8): undefined reference to `no symbol'
> | lib/crypto/arc4.o: In function `__ksymtab_arc4_crypt':
> | arc4.c:(___ksymtab+arc4_crypt+0x8): undefined reference to `no symbol'
>
>This is because the dummy initialisation of the 'namespace_offset' field
>in 'struct kernel_symbol' when using EXPORT_SYMBOL on architectures with
>support for PREL32 locations uses an offset from an absolute address (0)
>in an effort to trick 'offset_to_pointer' into behaving as a NOP,
>allowing non-namespaced symbols to be treated in the same way as those
>belonging to a namespace.
>
>Unfortunately, place-relative relocations require a symbol reference
>rather than an absolute value and, although x86 appears to get away with
>this due to placing the kernel text at the top of the address space, it
>almost certainly results in a runtime failure if the kernel is relocated
>dynamically as a result of KASLR.
>
>Rework 'namespace_offset' so that a value of 0, which cannot occur for a
>valid namespaced symbol, indicates that the corresponding symbol does
>not belong to a namespace.
>
>Cc: Matthias Maennich <maennich@google.com>
>Cc: Jessica Yu <jeyu@kernel.org>
>Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>Cc: Catalin Marinas <catalin.marinas@arm.com>
>Fixes: 8651ec01daed ("module: add support for symbol namespaces.")
>Reported-by: kbuild test robot <lkp@intel.com>
>Signed-off-by: Will Deacon <will@kernel.org>
>---
>
>Please note that I've not been able to test this at LPC, but it's been
>submitted to kernelci.
Thanks for fixing this so quickly. I can confirm that this fixes the
build for arm64 defconfig and x86 built fine for me as well. I'll wait
a bit and apply this at the end of the day in case Matthias or anybody
else would like to confirm/test.
Thanks,
Jessica
> include/asm-generic/export.h | 2 +-
> include/linux/export.h | 2 +-
> kernel/module.c | 2 ++
> 3 files changed, 4 insertions(+), 2 deletions(-)
>
>diff --git a/include/asm-generic/export.h b/include/asm-generic/export.h
>index e2b5d0f569d3..d0912c7ac2fc 100644
>--- a/include/asm-generic/export.h
>+++ b/include/asm-generic/export.h
>@@ -17,7 +17,7 @@
>
> .macro __put, val, name
> #ifdef CONFIG_HAVE_ARCH_PREL32_RELOCATIONS
>- .long \val - ., \name - ., 0 - .
>+ .long \val - ., \name - ., 0
> #elif defined(CONFIG_64BIT)
> .quad \val, \name, 0
> #else
>diff --git a/include/linux/export.h b/include/linux/export.h
>index 2c5468d8ea9a..ef5d015d754a 100644
>--- a/include/linux/export.h
>+++ b/include/linux/export.h
>@@ -68,7 +68,7 @@ extern struct module __this_module;
> "__ksymtab_" #sym ": \n" \
> " .long " #sym "- . \n" \
> " .long __kstrtab_" #sym "- . \n" \
>- " .long 0 - . \n" \
>+ " .long 0 \n" \
> " .previous \n")
>
> struct kernel_symbol {
>diff --git a/kernel/module.c b/kernel/module.c
>index f76efcf2043e..7ab244c4e1ba 100644
>--- a/kernel/module.c
>+++ b/kernel/module.c
>@@ -547,6 +547,8 @@ static const char *kernel_symbol_name(const struct kernel_symbol *sym)
> static const char *kernel_symbol_namespace(const struct kernel_symbol *sym)
> {
> #ifdef CONFIG_HAVE_ARCH_PREL32_RELOCATIONS
>+ if (!sym->namespace_offset)
>+ return NULL;
> return offset_to_ptr(&sym->namespace_offset);
> #else
> return sym->namespace;
>--
>2.23.0.162.g0b9fbb3734-goog
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] module: Fix link failure due to invalid relocation on namespace offset
2019-09-11 13:35 ` Jessica Yu
@ 2019-09-11 16:40 ` Will Deacon
2019-09-11 16:53 ` Matthias Maennich
0 siblings, 1 reply; 6+ messages in thread
From: Will Deacon @ 2019-09-11 16:40 UTC (permalink / raw)
To: Jessica Yu
Cc: linux-kernel, maco, gregkh, Matthias Maennich, Ard Biesheuvel,
Catalin Marinas
On Wed, Sep 11, 2019 at 03:35:06PM +0200, Jessica Yu wrote:
> +++ Will Deacon [11/09/19 13:26 +0100]:
> > Commit 8651ec01daed ("module: add support for symbol namespaces.")
> > broke linking for arm64 defconfig:
> >
> > | lib/crypto/arc4.o: In function `__ksymtab_arc4_setkey':
> > | arc4.c:(___ksymtab+arc4_setkey+0x8): undefined reference to `no symbol'
> > | lib/crypto/arc4.o: In function `__ksymtab_arc4_crypt':
> > | arc4.c:(___ksymtab+arc4_crypt+0x8): undefined reference to `no symbol'
> >
> > This is because the dummy initialisation of the 'namespace_offset' field
> > in 'struct kernel_symbol' when using EXPORT_SYMBOL on architectures with
> > support for PREL32 locations uses an offset from an absolute address (0)
> > in an effort to trick 'offset_to_pointer' into behaving as a NOP,
> > allowing non-namespaced symbols to be treated in the same way as those
> > belonging to a namespace.
> >
> > Unfortunately, place-relative relocations require a symbol reference
> > rather than an absolute value and, although x86 appears to get away with
> > this due to placing the kernel text at the top of the address space, it
> > almost certainly results in a runtime failure if the kernel is relocated
> > dynamically as a result of KASLR.
> >
> > Rework 'namespace_offset' so that a value of 0, which cannot occur for a
> > valid namespaced symbol, indicates that the corresponding symbol does
> > not belong to a namespace.
> >
> > Cc: Matthias Maennich <maennich@google.com>
> > Cc: Jessica Yu <jeyu@kernel.org>
> > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > Cc: Catalin Marinas <catalin.marinas@arm.com>
> > Fixes: 8651ec01daed ("module: add support for symbol namespaces.")
> > Reported-by: kbuild test robot <lkp@intel.com>
> > Signed-off-by: Will Deacon <will@kernel.org>
> > ---
> >
> > Please note that I've not been able to test this at LPC, but it's been
> > submitted to kernelci.
>
> Thanks for fixing this so quickly. I can confirm that this fixes the
> build for arm64 defconfig and x86 built fine for me as well. I'll wait
> a bit and apply this at the end of the day in case Matthias or anybody
> else would like to confirm/test.
FWIW, I've managed to boot arm64 Debian under QEMU and load/unload
modules successfully with this patch applied on top of modules-next.
Will
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] module: Fix link failure due to invalid relocation on namespace offset
2019-09-11 16:40 ` Will Deacon
@ 2019-09-11 16:53 ` Matthias Maennich
0 siblings, 0 replies; 6+ messages in thread
From: Matthias Maennich @ 2019-09-11 16:53 UTC (permalink / raw)
To: Will Deacon
Cc: Jessica Yu, linux-kernel, maco, gregkh, Ard Biesheuvel,
Catalin Marinas, kernel-team
On Wed, Sep 11, 2019 at 05:40:13PM +0100, Will Deacon wrote:
>On Wed, Sep 11, 2019 at 03:35:06PM +0200, Jessica Yu wrote:
>> +++ Will Deacon [11/09/19 13:26 +0100]:
>> > Commit 8651ec01daed ("module: add support for symbol namespaces.")
>> > broke linking for arm64 defconfig:
>> >
>> > | lib/crypto/arc4.o: In function `__ksymtab_arc4_setkey':
>> > | arc4.c:(___ksymtab+arc4_setkey+0x8): undefined reference to `no symbol'
>> > | lib/crypto/arc4.o: In function `__ksymtab_arc4_crypt':
>> > | arc4.c:(___ksymtab+arc4_crypt+0x8): undefined reference to `no symbol'
>> >
>> > This is because the dummy initialisation of the 'namespace_offset' field
>> > in 'struct kernel_symbol' when using EXPORT_SYMBOL on architectures with
>> > support for PREL32 locations uses an offset from an absolute address (0)
>> > in an effort to trick 'offset_to_pointer' into behaving as a NOP,
>> > allowing non-namespaced symbols to be treated in the same way as those
>> > belonging to a namespace.
>> >
>> > Unfortunately, place-relative relocations require a symbol reference
>> > rather than an absolute value and, although x86 appears to get away with
>> > this due to placing the kernel text at the top of the address space, it
>> > almost certainly results in a runtime failure if the kernel is relocated
>> > dynamically as a result of KASLR.
>> >
>> > Rework 'namespace_offset' so that a value of 0, which cannot occur for a
>> > valid namespaced symbol, indicates that the corresponding symbol does
>> > not belong to a namespace.
>> >
>> > Cc: Matthias Maennich <maennich@google.com>
>> > Cc: Jessica Yu <jeyu@kernel.org>
>> > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> > Cc: Catalin Marinas <catalin.marinas@arm.com>
>> > Fixes: 8651ec01daed ("module: add support for symbol namespaces.")
>> > Reported-by: kbuild test robot <lkp@intel.com>
>> > Signed-off-by: Will Deacon <will@kernel.org>
>> > ---
>> >
>> > Please note that I've not been able to test this at LPC, but it's been
>> > submitted to kernelci.
>>
>> Thanks for fixing this so quickly. I can confirm that this fixes the
>> build for arm64 defconfig and x86 built fine for me as well. I'll wait
>> a bit and apply this at the end of the day in case Matthias or anybody
>> else would like to confirm/test.
>
>FWIW, I've managed to boot arm64 Debian under QEMU and load/unload
>modules successfully with this patch applied on top of modules-next.
Thanks Will for fixing this so quickly! The patch looks good to me.
Feel free to add
Reviewed-by: Matthias Maennich <maennich@google.com>
Tested-by: Matthias Maennich <maennich@google.com>
Cheers,
Matthias
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] module: Fix link failure due to invalid relocation on namespace offset
2019-09-11 12:26 [PATCH] module: Fix link failure due to invalid relocation on namespace offset Will Deacon
2019-09-11 13:35 ` Jessica Yu
@ 2019-09-11 16:51 ` Ard Biesheuvel
2019-09-11 18:26 ` Jessica Yu
2 siblings, 0 replies; 6+ messages in thread
From: Ard Biesheuvel @ 2019-09-11 16:51 UTC (permalink / raw)
To: Will Deacon
Cc: Linux Kernel Mailing List, Martijn Coenen, Greg Kroah-Hartman,
Matthias Maennich, Jessica Yu, Catalin Marinas
On Wed, 11 Sep 2019 at 13:26, Will Deacon <will@kernel.org> wrote:
>
> Commit 8651ec01daed ("module: add support for symbol namespaces.")
> broke linking for arm64 defconfig:
>
> | lib/crypto/arc4.o: In function `__ksymtab_arc4_setkey':
> | arc4.c:(___ksymtab+arc4_setkey+0x8): undefined reference to `no symbol'
> | lib/crypto/arc4.o: In function `__ksymtab_arc4_crypt':
> | arc4.c:(___ksymtab+arc4_crypt+0x8): undefined reference to `no symbol'
>
> This is because the dummy initialisation of the 'namespace_offset' field
> in 'struct kernel_symbol' when using EXPORT_SYMBOL on architectures with
> support for PREL32 locations uses an offset from an absolute address (0)
> in an effort to trick 'offset_to_pointer' into behaving as a NOP,
> allowing non-namespaced symbols to be treated in the same way as those
> belonging to a namespace.
>
> Unfortunately, place-relative relocations require a symbol reference
> rather than an absolute value and, although x86 appears to get away with
> this due to placing the kernel text at the top of the address space, it
> almost certainly results in a runtime failure if the kernel is relocated
> dynamically as a result of KASLR.
>
> Rework 'namespace_offset' so that a value of 0, which cannot occur for a
> valid namespaced symbol, indicates that the corresponding symbol does
> not belong to a namespace.
>
> Cc: Matthias Maennich <maennich@google.com>
> Cc: Jessica Yu <jeyu@kernel.org>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Fixes: 8651ec01daed ("module: add support for symbol namespaces.")
> Reported-by: kbuild test robot <lkp@intel.com>
> Signed-off-by: Will Deacon <will@kernel.org>
> ---
>
> Please note that I've not been able to test this at LPC, but it's been
> submitted to kernelci.
>
The build tests all passed, and the boot tests that have completed by
now all look fine, so
Acked-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Tested-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> include/asm-generic/export.h | 2 +-
> include/linux/export.h | 2 +-
> kernel/module.c | 2 ++
> 3 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/include/asm-generic/export.h b/include/asm-generic/export.h
> index e2b5d0f569d3..d0912c7ac2fc 100644
> --- a/include/asm-generic/export.h
> +++ b/include/asm-generic/export.h
> @@ -17,7 +17,7 @@
>
> .macro __put, val, name
> #ifdef CONFIG_HAVE_ARCH_PREL32_RELOCATIONS
> - .long \val - ., \name - ., 0 - .
> + .long \val - ., \name - ., 0
> #elif defined(CONFIG_64BIT)
> .quad \val, \name, 0
> #else
> diff --git a/include/linux/export.h b/include/linux/export.h
> index 2c5468d8ea9a..ef5d015d754a 100644
> --- a/include/linux/export.h
> +++ b/include/linux/export.h
> @@ -68,7 +68,7 @@ extern struct module __this_module;
> "__ksymtab_" #sym ": \n" \
> " .long " #sym "- . \n" \
> " .long __kstrtab_" #sym "- . \n" \
> - " .long 0 - . \n" \
> + " .long 0 \n" \
> " .previous \n")
>
> struct kernel_symbol {
> diff --git a/kernel/module.c b/kernel/module.c
> index f76efcf2043e..7ab244c4e1ba 100644
> --- a/kernel/module.c
> +++ b/kernel/module.c
> @@ -547,6 +547,8 @@ static const char *kernel_symbol_name(const struct kernel_symbol *sym)
> static const char *kernel_symbol_namespace(const struct kernel_symbol *sym)
> {
> #ifdef CONFIG_HAVE_ARCH_PREL32_RELOCATIONS
> + if (!sym->namespace_offset)
> + return NULL;
> return offset_to_ptr(&sym->namespace_offset);
> #else
> return sym->namespace;
> --
> 2.23.0.162.g0b9fbb3734-goog
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] module: Fix link failure due to invalid relocation on namespace offset
2019-09-11 12:26 [PATCH] module: Fix link failure due to invalid relocation on namespace offset Will Deacon
2019-09-11 13:35 ` Jessica Yu
2019-09-11 16:51 ` Ard Biesheuvel
@ 2019-09-11 18:26 ` Jessica Yu
2 siblings, 0 replies; 6+ messages in thread
From: Jessica Yu @ 2019-09-11 18:26 UTC (permalink / raw)
To: Will Deacon
Cc: linux-kernel, maco, gregkh, Matthias Maennich, Ard Biesheuvel,
Catalin Marinas
+++ Will Deacon [11/09/19 13:26 +0100]:
>Commit 8651ec01daed ("module: add support for symbol namespaces.")
>broke linking for arm64 defconfig:
>
> | lib/crypto/arc4.o: In function `__ksymtab_arc4_setkey':
> | arc4.c:(___ksymtab+arc4_setkey+0x8): undefined reference to `no symbol'
> | lib/crypto/arc4.o: In function `__ksymtab_arc4_crypt':
> | arc4.c:(___ksymtab+arc4_crypt+0x8): undefined reference to `no symbol'
>
>This is because the dummy initialisation of the 'namespace_offset' field
>in 'struct kernel_symbol' when using EXPORT_SYMBOL on architectures with
>support for PREL32 locations uses an offset from an absolute address (0)
>in an effort to trick 'offset_to_pointer' into behaving as a NOP,
>allowing non-namespaced symbols to be treated in the same way as those
>belonging to a namespace.
>
>Unfortunately, place-relative relocations require a symbol reference
>rather than an absolute value and, although x86 appears to get away with
>this due to placing the kernel text at the top of the address space, it
>almost certainly results in a runtime failure if the kernel is relocated
>dynamically as a result of KASLR.
>
>Rework 'namespace_offset' so that a value of 0, which cannot occur for a
>valid namespaced symbol, indicates that the corresponding symbol does
>not belong to a namespace.
>
>Cc: Matthias Maennich <maennich@google.com>
>Cc: Jessica Yu <jeyu@kernel.org>
>Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>Cc: Catalin Marinas <catalin.marinas@arm.com>
>Fixes: 8651ec01daed ("module: add support for symbol namespaces.")
>Reported-by: kbuild test robot <lkp@intel.com>
>Signed-off-by: Will Deacon <will@kernel.org>
Applied, thanks everyone!
Jessica
>---
>
>Please note that I've not been able to test this at LPC, but it's been
>submitted to kernelci.
>
> include/asm-generic/export.h | 2 +-
> include/linux/export.h | 2 +-
> kernel/module.c | 2 ++
> 3 files changed, 4 insertions(+), 2 deletions(-)
>
>diff --git a/include/asm-generic/export.h b/include/asm-generic/export.h
>index e2b5d0f569d3..d0912c7ac2fc 100644
>--- a/include/asm-generic/export.h
>+++ b/include/asm-generic/export.h
>@@ -17,7 +17,7 @@
>
> .macro __put, val, name
> #ifdef CONFIG_HAVE_ARCH_PREL32_RELOCATIONS
>- .long \val - ., \name - ., 0 - .
>+ .long \val - ., \name - ., 0
> #elif defined(CONFIG_64BIT)
> .quad \val, \name, 0
> #else
>diff --git a/include/linux/export.h b/include/linux/export.h
>index 2c5468d8ea9a..ef5d015d754a 100644
>--- a/include/linux/export.h
>+++ b/include/linux/export.h
>@@ -68,7 +68,7 @@ extern struct module __this_module;
> "__ksymtab_" #sym ": \n" \
> " .long " #sym "- . \n" \
> " .long __kstrtab_" #sym "- . \n" \
>- " .long 0 - . \n" \
>+ " .long 0 \n" \
> " .previous \n")
>
> struct kernel_symbol {
>diff --git a/kernel/module.c b/kernel/module.c
>index f76efcf2043e..7ab244c4e1ba 100644
>--- a/kernel/module.c
>+++ b/kernel/module.c
>@@ -547,6 +547,8 @@ static const char *kernel_symbol_name(const struct kernel_symbol *sym)
> static const char *kernel_symbol_namespace(const struct kernel_symbol *sym)
> {
> #ifdef CONFIG_HAVE_ARCH_PREL32_RELOCATIONS
>+ if (!sym->namespace_offset)
>+ return NULL;
> return offset_to_ptr(&sym->namespace_offset);
> #else
> return sym->namespace;
>--
>2.23.0.162.g0b9fbb3734-goog
>
^ permalink raw reply [flat|nested] 6+ messages in thread