diff mbox

arm64: kaslr: fix breakage with CONFIG_MODVERSIONS=y

Message ID 1476376929-29688-1-git-send-email-ard.biesheuvel@linaro.org
State Accepted
Commit 9c0e83c371cf4696926c95f9c8c77cd6ea803426
Headers show

Commit Message

Ard Biesheuvel Oct. 13, 2016, 4:42 p.m. UTC
As it turns out, the KASLR code breaks CONFIG_MODVERSIONS, since the
kcrctab has an absolute address field that is relocated at runtime
when the kernel offset is randomized.

This has been fixed already for PowerPC in the past, so simply wire up
the existing code dealing with this issue.

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

---

Given that I spotted this when trying to boot a distro kernel, this probably
deserves a cc stable.

Timur, could you please test this and report back? Thanks.

 arch/arm64/include/asm/module.h | 5 +++++
 1 file changed, 5 insertions(+)

-- 
2.7.4


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

Comments

Ard Biesheuvel Oct. 14, 2016, 6:23 p.m. UTC | #1
On 13 October 2016 at 20:59, Timur Tabi <timur@codeaurora.org> wrote:
> Ard Biesheuvel wrote:

>>

>> As it turns out, the KASLR code breaks CONFIG_MODVERSIONS, since the

>> kcrctab has an absolute address field that is relocated at runtime

>> when the kernel offset is randomized.

>>

>> This has been fixed already for PowerPC in the past, so simply wire up

>> the existing code dealing with this issue.

>>

>> Signed-off-by: Ard Biesheuvel<ard.biesheuvel@linaro.org>

>

>

> Tested-by: Timur Tabi <timur@codeaurora.org>

>


Thanks. I will resend this with a fixes: tag and a better description

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Will Deacon Oct. 14, 2016, 6:26 p.m. UTC | #2
On Fri, Oct 14, 2016 at 07:23:15PM +0100, Ard Biesheuvel wrote:
> On 13 October 2016 at 20:59, Timur Tabi <timur@codeaurora.org> wrote:

> > Ard Biesheuvel wrote:

> >>

> >> As it turns out, the KASLR code breaks CONFIG_MODVERSIONS, since the

> >> kcrctab has an absolute address field that is relocated at runtime

> >> when the kernel offset is randomized.

> >>

> >> This has been fixed already for PowerPC in the past, so simply wire up

> >> the existing code dealing with this issue.

> >>

> >> Signed-off-by: Ard Biesheuvel<ard.biesheuvel@linaro.org>

> >

> >

> > Tested-by: Timur Tabi <timur@codeaurora.org>

> >

> 

> Thanks. I will resend this with a fixes: tag and a better description


Feel free, but I already queued it locally and added the Fixes tag myself.
I'm just waiting for Lorenzo to post a fix to the ACPI NUMA stuff, then
I'll send these two up together next week.
Will

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Ard Biesheuvel Oct. 14, 2016, 7:53 p.m. UTC | #3
On 14 October 2016 at 19:26, Will Deacon <will.deacon@arm.com> wrote:
> On Fri, Oct 14, 2016 at 07:23:15PM +0100, Ard Biesheuvel wrote:

>> On 13 October 2016 at 20:59, Timur Tabi <timur@codeaurora.org> wrote:

>> > Ard Biesheuvel wrote:

>> >>

>> >> As it turns out, the KASLR code breaks CONFIG_MODVERSIONS, since the

>> >> kcrctab has an absolute address field that is relocated at runtime

>> >> when the kernel offset is randomized.

>> >>

>> >> This has been fixed already for PowerPC in the past, so simply wire up

>> >> the existing code dealing with this issue.

>> >>

>> >> Signed-off-by: Ard Biesheuvel<ard.biesheuvel@linaro.org>

>> >

>> >

>> > Tested-by: Timur Tabi <timur@codeaurora.org>

>> >

>>

>> Thanks. I will resend this with a fixes: tag and a better description

>

> Feel free, but I already queued it locally and added the Fixes tag myself.

> I'm just waiting for Lorenzo to post a fix to the ACPI NUMA stuff, then

> I'll send these two up together next week.


It's no big deal. The description is not entirely accurate in the
sense that the kcrctab does not contain an absolute address field, but
it masquerades as an absolute address so that the build system can
populate the kcrctab entries using a linker script include containing
name=value pairs. This does not only result in 4 wasted bytes per CRC,
but on PPC64 and arm64 with CONFIG_RELOCATABLE=y, it also results in
the breakage this patch addresses, and more importantly, results in a
24 byte RELA entry per CRC in the __init section. So I intend to
propose a patch to change this in the generic code, after which this
patch could be reverted.

BTW, I spotted another KASLR issue, with ftrace this time, where it
attempts to poke relative branches into modules targeting the core
kernel, which is likely to fail when
CONFIG_RANDOMIZE_MODULE_REGION_FULL=y. Should we address this at the
Kconfig level? Or should we try to fix ftrace to support long
branches?

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Will Deacon Oct. 17, 2016, 2:48 p.m. UTC | #4
On Fri, Oct 14, 2016 at 08:53:02PM +0100, Ard Biesheuvel wrote:
> On 14 October 2016 at 19:26, Will Deacon <will.deacon@arm.com> wrote:

> > On Fri, Oct 14, 2016 at 07:23:15PM +0100, Ard Biesheuvel wrote:

> >> On 13 October 2016 at 20:59, Timur Tabi <timur@codeaurora.org> wrote:

> >> > Ard Biesheuvel wrote:

> >> >>

> >> >> As it turns out, the KASLR code breaks CONFIG_MODVERSIONS, since the

> >> >> kcrctab has an absolute address field that is relocated at runtime

> >> >> when the kernel offset is randomized.

> >> >>

> >> >> This has been fixed already for PowerPC in the past, so simply wire up

> >> >> the existing code dealing with this issue.

> >> >>

> >> >> Signed-off-by: Ard Biesheuvel<ard.biesheuvel@linaro.org>

> >> >

> >> >

> >> > Tested-by: Timur Tabi <timur@codeaurora.org>

> >> >

> >>

> >> Thanks. I will resend this with a fixes: tag and a better description

> >

> > Feel free, but I already queued it locally and added the Fixes tag myself.

> > I'm just waiting for Lorenzo to post a fix to the ACPI NUMA stuff, then

> > I'll send these two up together next week.

> 

> It's no big deal. The description is not entirely accurate in the

> sense that the kcrctab does not contain an absolute address field, but

> it masquerades as an absolute address so that the build system can

> populate the kcrctab entries using a linker script include containing

> name=value pairs. This does not only result in 4 wasted bytes per CRC,

> but on PPC64 and arm64 with CONFIG_RELOCATABLE=y, it also results in

> the breakage this patch addresses, and more importantly, results in a

> 24 byte RELA entry per CRC in the __init section. So I intend to

> propose a patch to change this in the generic code, after which this

> patch could be reverted.

> 

> BTW, I spotted another KASLR issue, with ftrace this time, where it

> attempts to poke relative branches into modules targeting the core

> kernel, which is likely to fail when

> CONFIG_RANDOMIZE_MODULE_REGION_FULL=y. Should we address this at the

> Kconfig level? Or should we try to fix ftrace to support long

> branches?


I guess we could fix it at the kconfig level in the short term, then it
makes it clear that some ftrace work needs doing to fix it properly.

Will

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

Patch

diff --git a/arch/arm64/include/asm/module.h b/arch/arm64/include/asm/module.h
index e12af6754634..06ff7fd9e81f 100644
--- a/arch/arm64/include/asm/module.h
+++ b/arch/arm64/include/asm/module.h
@@ -17,6 +17,7 @@ 
 #define __ASM_MODULE_H
 
 #include <asm-generic/module.h>
+#include <asm/memory.h>
 
 #define MODULE_ARCH_VERMAGIC	"aarch64"
 
@@ -32,6 +33,10 @@  u64 module_emit_plt_entry(struct module *mod, const Elf64_Rela *rela,
 			  Elf64_Sym *sym);
 
 #ifdef CONFIG_RANDOMIZE_BASE
+#ifdef CONFIG_MODVERSIONS
+#define ARCH_RELOCATES_KCRCTAB
+#define reloc_start 		(kimage_vaddr - KIMAGE_VADDR)
+#endif
 extern u64 module_alloc_base;
 #else
 #define module_alloc_base	((u64)_etext - MODULES_VSIZE)