diff mbox

ARM: vmlinux.lds: assert that ROM and RAM don't overlap when XIP_KERNEL=y

Message ID 1454346154-12931-1-git-send-email-ard.biesheuvel@linaro.org (mailing list archive)
State New, archived
Headers show

Commit Message

Ard Biesheuvel Feb. 1, 2016, 5:02 p.m. UTC
When building an XIP kernel, the linker produces two disjoint VMA regions,
where the first is mapped onto ROM and the second onto RAM. For this reason,
the linker output pointer '.' is updated halfway through the linker script,
and set to a value that corresponds with the start of the RAM region.

However, in some cases, the ROM region exceeds the expected size, and the
assignment of the output pointer results in a decrement rather than an
increment, causing the virtual addresses of the .data region to clash with
the .text region. Such a kernel cannot boot normally, but it also confuses
the hell out of kallsyms, since .data symbols may appear inside the
[_stext, _etext] or [_sinittext, _einittext] intervals in the first pass,
but not in the second (or vice versa), resulting in inconsistent kallsyms
data.

So let's make sure that the output pointer only advances, and never jumps
back into the ROM region.

Cc: arnd@arndb.de
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 arch/arm/kernel/vmlinux.lds.S | 1 +
 1 file changed, 1 insertion(+)

Comments

Arnd Bergmann Feb. 1, 2016, 7:22 p.m. UTC | #1
On Monday 01 February 2016 18:02:34 Ard Biesheuvel wrote:
> When building an XIP kernel, the linker produces two disjoint VMA regions,
> where the first is mapped onto ROM and the second onto RAM. For this reason,
> the linker output pointer '.' is updated halfway through the linker script,
> and set to a value that corresponds with the start of the RAM region.
> 
> However, in some cases, the ROM region exceeds the expected size, and the
> assignment of the output pointer results in a decrement rather than an
> increment, causing the virtual addresses of the .data region to clash with
> the .text region. Such a kernel cannot boot normally, but it also confuses
> the hell out of kallsyms, since .data symbols may appear inside the
> [_stext, _etext] or [_sinittext, _einittext] intervals in the first pass,
> but not in the second (or vice versa), resulting in inconsistent kallsyms
> data.
> 
> So let's make sure that the output pointer only advances, and never jumps
> back into the ROM region.
> 
> Cc: arnd@arndb.de
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> 

Acked-by: Arnd Bergmann <arnd@arndb.de>

I've tested this in multiple configurations, and it reliably prints
a useful error message in broken configurations, while it has no
effect on working configurations. This is more helpful than the
mysterious

   Inconsistent kallsyms data
   Try make KALLSYMS_EXTRA_PASS=1 as a workaround

message that we get without the patch.

	Arnd
Russell King - ARM Linux Feb. 3, 2016, 12:31 a.m. UTC | #2
On Mon, Feb 01, 2016 at 06:02:34PM +0100, Ard Biesheuvel wrote:
> When building an XIP kernel, the linker produces two disjoint VMA regions,
> where the first is mapped onto ROM and the second onto RAM. For this reason,
> the linker output pointer '.' is updated halfway through the linker script,
> and set to a value that corresponds with the start of the RAM region.
> 
> However, in some cases, the ROM region exceeds the expected size, and the
> assignment of the output pointer results in a decrement rather than an
> increment, causing the virtual addresses of the .data region to clash with
> the .text region. Such a kernel cannot boot normally, but it also confuses
> the hell out of kallsyms, since .data symbols may appear inside the
> [_stext, _etext] or [_sinittext, _einittext] intervals in the first pass,
> but not in the second (or vice versa), resulting in inconsistent kallsyms
> data.
> 
> So let's make sure that the output pointer only advances, and never jumps
> back into the ROM region.

The long term goal is to move the XIP stuff out of this file, so
I think it's better to avoid touching this until the split has
happened.

I'd _much_ rather see the split happen first, and then fixes
applied, rather than trying to fix the existing unmaintainable
mess.
Jared Hulbert Feb. 3, 2016, 3:06 a.m. UTC | #3
On Tue, Feb 2, 2016 at 4:31 PM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Mon, Feb 01, 2016 at 06:02:34PM +0100, Ard Biesheuvel wrote:
>> When building an XIP kernel, the linker produces two disjoint VMA regions,
>> where the first is mapped onto ROM and the second onto RAM. For this reason,
>> the linker output pointer '.' is updated halfway through the linker script,
>> and set to a value that corresponds with the start of the RAM region.
>>
>> However, in some cases, the ROM region exceeds the expected size, and the
>> assignment of the output pointer results in a decrement rather than an
>> increment, causing the virtual addresses of the .data region to clash with
>> the .text region. Such a kernel cannot boot normally, but it also confuses
>> the hell out of kallsyms, since .data symbols may appear inside the
>> [_stext, _etext] or [_sinittext, _einittext] intervals in the first pass,
>> but not in the second (or vice versa), resulting in inconsistent kallsyms
>> data.
>>
>> So let's make sure that the output pointer only advances, and never jumps
>> back into the ROM region.
>
> The long term goal is to move the XIP stuff out of this file, so
> I think it's better to avoid touching this until the split has
> happened.
>
> I'd _much_ rather see the split happen first, and then fixes
> applied, rather than trying to fix the existing unmaintainable
> mess.

Is that split being actively worked on?  If so by whom?
Chris Brandt Feb. 3, 2016, 3:54 a.m. UTC | #4
> > The long term goal is to move the XIP stuff out of this file, so I 
> > think it's better to avoid touching this until the split has happened.
> >
> > I'd _much_ rather see the split happen first, and then fixes applied, 
> > rather than trying to fix the existing unmaintainable mess.
>
> Is that split being actively worked on?  If so by whom?

I submitted "[PATCH v2] ARM: xip: Move XIP linking to a separate file" back in November which seemed pretty straight forward.

Although, it didn't make it too far (other than Nicolas acking it)

Chris
Nicolas Pitre Feb. 3, 2016, 4:08 a.m. UTC | #5
On Wed, 3 Feb 2016, Chris Brandt wrote:

> > > The long term goal is to move the XIP stuff out of this file, so I 
> > > think it's better to avoid touching this until the split has happened.
> > >
> > > I'd _much_ rather see the split happen first, and then fixes applied, 
> > > rather than trying to fix the existing unmaintainable mess.
> >
> > Is that split being actively worked on?  If so by whom?
> 
> I submitted "[PATCH v2] ARM: xip: Move XIP linking to a separate file" back in November which seemed pretty straight forward.
> 
> Although, it didn't make it too far (other than Nicolas acking it)

For your ARM patches to be merged upstream, once you feel they've 
received sufficient review on the list, you then have to submit them 
here:

http://www.arm.linux.org.uk/developer/patches/


Nicolas
diff mbox

Patch

diff --git a/arch/arm/kernel/vmlinux.lds.S b/arch/arm/kernel/vmlinux.lds.S
index 8b60fde5ce48..950114d533e1 100644
--- a/arch/arm/kernel/vmlinux.lds.S
+++ b/arch/arm/kernel/vmlinux.lds.S
@@ -229,6 +229,7 @@  SECTIONS
 
 #ifdef CONFIG_XIP_KERNEL
 	__data_loc = ALIGN(4);		/* location in binary */
+	ASSERT(. < PAGE_OFFSET + TEXT_OFFSET, "XIP_KERNEL: ROM and RAM overlap")
 	. = PAGE_OFFSET + TEXT_OFFSET;
 #else
 #ifdef CONFIG_ARM_KERNMEM_PERMS