linux-kernel - Re: [PATCH] module: Fix link failure due to invalid relocation on namespace offset

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20190911133506.GB7837@linux-8ccs>
Date:   Wed, 11 Sep 2019 15:35:06 +0200
From:   Jessica Yu <jeyu@...nel.org>
To:     Will Deacon <will@...nel.org>
Cc:     linux-kernel@...r.kernel.org, maco@...roid.com,
        gregkh@...uxfoundation.org,
        Matthias Maennich <maennich@...gle.com>,
        Ard Biesheuvel <ard.biesheuvel@...aro.org>,
        Catalin Marinas <catalin.marinas@....com>
Subject: Re: [PATCH] module: Fix link failure due to invalid relocation on
 namespace offset

+++ 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@...gle.com>
>Cc: Jessica Yu <jeyu@...nel.org>
>Cc: Ard Biesheuvel <ard.biesheuvel@...aro.org>
>Cc: Catalin Marinas <catalin.marinas@....com>
>Fixes: 8651ec01daed ("module: add support for symbol namespaces.")
>Reported-by: kbuild test robot <lkp@...el.com>
>Signed-off-by: Will Deacon <will@...nel.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
>