arm64: vmlinux.lds: make __rela_offset and __dynsym_offset ABSOLUTE

PATCH New Ard Biesheuvel 2 participants 8 messages Jul 28, 2016 – Jul 29, 2016 arm64

Share & tools

Ard Biesheuvel

author

2016-07-28 14:15 UTC #0 lore raw

PATCH 1/1 arm64: vmlinux.lds: make __rela_offset and __dynsym_offset ABSOLUTE

Due to the untyped KIMAGE_VADDR constant, the linker may not notice that the __rela_offset and __dynsym_offset expressions are absolute values (i.e., are not subject to relocation). This does not matter for KASLR, but it does confuse kallsyms in relative mode, since it uses the lowest non-absolute symbol address as the anchor point, and expects all other symbol addresses to be within 4 GB of it.

Fix this by qualifying these expressions as ABSOLUTE() explicitly.

Fixes 0cd3defe0af4 ("arm64: kernel: perform relocation processing from ID map")

Signed-off-by Ard Biesheuvel <[email protected]>

---
 arch/arm64/kernel/vmlinux.lds.S | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

arch/arm64/kernel/vmlinux.lds.S +2 -2

index 89d6e177ecbd..d49492179967 100644
@@ -184,9 +184,9 @@ SECTIONS
 		*(.hash)
 	}
 
-	__rela_offset	= ADDR(.rela) - KIMAGE_VADDR;
+	__rela_offset	= ABSOLUTE(ADDR(.rela) - KIMAGE_VADDR);
 	__rela_size	= SIZEOF(.rela);
-	__dynsym_offset	= ADDR(.dynsym) - KIMAGE_VADDR;
+	__dynsym_offset	= ABSOLUTE(ADDR(.dynsym) - KIMAGE_VADDR);
 
 	. = ALIGN(SEGMENT_ALIGN);
 	__init_end = .;

Signature · 1 line

2.7.4

Will Deacon

2016-07-29 09:06 UTC #1 lore raw

PATCH 1/1 arm64: vmlinux.lds: make __rela_offset and __dynsym_offset ABSOLUTE

Hi Ard,

Quoted 20 lines · Ard Biesheuvel

Due to the untyped KIMAGE_VADDR constant, the linker may not notice that the __rela_offset and __dynsym_offset expressions are absolute values (i.e., are not subject to relocation). This does not matter for KASLR, but it does confuse kallsyms in relative mode, since it uses the lowest non-absolute symbol address as the anchor point, and expects all other symbol addresses to be within 4 GB of it.
Fix this by qualifying these expressions as ABSOLUTE() explicitly.
Fixes 0cd3defe0af4 ("arm64: kernel: perform relocation processing from ID map")
Signed-off-by Ard Biesheuvel <[email protected]>
---
 arch/arm64/kernel/vmlinux.lds.S | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
arch/arm64/kernel/vmlinux.lds.S +0 -0
index 89d6e177ecbd..d49492179967 100644

arch/arm64/kernel/vmlinux.lds.S SECTIONS · line 189

Quoted 8 lines

arch/arm64/kernel/vmlinux.lds.S +2 -2

@@ -184,9 +184,9 @@ SECTIONS
 		*(.hash)
 	}
 
-	__rela_offset	= ADDR(.rela) - KIMAGE_VADDR;
+	__rela_offset	= ABSOLUTE(ADDR(.rela) - KIMAGE_VADDR);
 	__rela_size	= SIZEOF(.rela);
-	__dynsym_offset	= ADDR(.dynsym) - KIMAGE_VADDR;
+	__dynsym_offset	= ABSOLUTE(ADDR(.dynsym) - KIMAGE_VADDR);

You removed the __dynsym_offset entry entirely in "arm64: relocatable:
suppress R_AARCH64_ABS64 relocations in vmlinux", so I'll drop this part of the patch and assume you'll do a separate backport for stable. Ok?

Will

Ard Biesheuvel

author

2016-07-29 09:30 UTC #2 lore raw

PATCH 1/1 arm64: vmlinux.lds: make __rela_offset and __dynsym_offset ABSOLUTE

Quoted 26 lines · Will Deacon

Hi Ard,
Quoted 23 lines · Ard Biesheuvel
Due to the untyped KIMAGE_VADDR constant, the linker may not notice that the __rela_offset and __dynsym_offset expressions are absolute values (i.e., are not subject to relocation). This does not matter for KASLR, but it does confuse kallsyms in relative mode, since it uses the lowest non-absolute symbol address as the anchor point, and expects all other symbol addresses to be within 4 GB of it.
Fix this by qualifying these expressions as ABSOLUTE() explicitly.
Fixes 0cd3defe0af4 ("arm64: kernel: perform relocation processing from ID map")
Signed-off-by Ard Biesheuvel <[email protected]>
---
 arch/arm64/kernel/vmlinux.lds.S | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
arch/arm64/kernel/vmlinux.lds.S +0 -0
index 89d6e177ecbd..d49492179967 100644
@@ -184,9 +184,9 @@ SECTIONS
              *(.hash)
      }
 

Quoted 10 lines

arch/arm64/kernel/vmlinux.lds.S SECTIONS · line 189
Quoted 5 lines
arch/arm64/kernel/vmlinux.lds.S +2 -2
-     __rela_offset   = ADDR(.rela) - KIMAGE_VADDR;
+     __rela_offset   = ABSOLUTE(ADDR(.rela) - KIMAGE_VADDR);
      __rela_size     = SIZEOF(.rela);
-     __dynsym_offset = ADDR(.dynsym) - KIMAGE_VADDR;
+     __dynsym_offset = ABSOLUTE(ADDR(.dynsym) - KIMAGE_VADDR);
You removed the __dynsym_offset entry entirely in "arm64: relocatable:
suppress R_AARCH64_ABS64 relocations in vmlinux", so I'll drop this part of the patch and assume you'll do a separate backport for stable. Ok?

If you are ok with fixing up patches manually while applying, could we just reorder them instead?

Ard Biesheuvel

author

2016-07-29 10:46 UTC #3 lore raw

PATCH 1/1 arm64: vmlinux.lds: make __rela_offset and __dynsym_offset ABSOLUTE

Quoted 38 lines · Ard Biesheuvel

Quoted 26 lines · Will Deacon
Hi Ard,
Quoted 23 lines · Ard Biesheuvel
Due to the untyped KIMAGE_VADDR constant, the linker may not notice that the __rela_offset and __dynsym_offset expressions are absolute values (i.e., are not subject to relocation). This does not matter for KASLR, but it does confuse kallsyms in relative mode, since it uses the lowest non-absolute symbol address as the anchor point, and expects all other symbol addresses to be within 4 GB of it.
Fix this by qualifying these expressions as ABSOLUTE() explicitly.
Fixes 0cd3defe0af4 ("arm64: kernel: perform relocation processing from ID map")
Signed-off-by Ard Biesheuvel <[email protected]>
---
 arch/arm64/kernel/vmlinux.lds.S | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
arch/arm64/kernel/vmlinux.lds.S +0 -0
index 89d6e177ecbd..d49492179967 100644
@@ -184,9 +184,9 @@ SECTIONS
              *(.hash)
      }
 
Quoted 10 lines
arch/arm64/kernel/vmlinux.lds.S SECTIONS · line 189
Quoted 5 lines
arch/arm64/kernel/vmlinux.lds.S +2 -2
-     __rela_offset   = ADDR(.rela) - KIMAGE_VADDR;
+     __rela_offset   = ABSOLUTE(ADDR(.rela) - KIMAGE_VADDR);
      __rela_size     = SIZEOF(.rela);
-     __dynsym_offset = ADDR(.dynsym) - KIMAGE_VADDR;
+     __dynsym_offset = ABSOLUTE(ADDR(.dynsym) - KIMAGE_VADDR);
You removed the __dynsym_offset entry entirely in "arm64: relocatable:
suppress R_AARCH64_ABS64 relocations in vmlinux", so I'll drop this part of the patch and assume you'll do a separate backport for stable. Ok?

Quoted 2 lines

If you are ok with fixing up patches manually while applying, could we just reorder them instead?

Actually, this should go into v4.8-late not -stable

Will Deacon

2016-07-29 10:49 UTC #4 lore raw

PATCH 1/1 arm64: vmlinux.lds: make __rela_offset and __dynsym_offset ABSOLUTE

Quoted 39 lines · Ard Biesheuvel

Quoted 38 lines · Ard Biesheuvel
Quoted 36 lines · Will Deacon
Hi Ard,
Quoted 20 lines · Ard Biesheuvel
Due to the untyped KIMAGE_VADDR constant, the linker may not notice that the __rela_offset and __dynsym_offset expressions are absolute values (i.e., are not subject to relocation). This does not matter for KASLR, but it does confuse kallsyms in relative mode, since it uses the lowest non-absolute symbol address as the anchor point, and expects all other symbol addresses to be within 4 GB of it.
Fix this by qualifying these expressions as ABSOLUTE() explicitly.
Fixes 0cd3defe0af4 ("arm64: kernel: perform relocation processing from ID map")
Signed-off-by Ard Biesheuvel <[email protected]>
---
 arch/arm64/kernel/vmlinux.lds.S | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
arch/arm64/kernel/vmlinux.lds.S +0 -0
index 89d6e177ecbd..d49492179967 100644
arch/arm64/kernel/vmlinux.lds.S SECTIONS · line 189
Quoted 8 lines
arch/arm64/kernel/vmlinux.lds.S +2 -2
@@ -184,9 +184,9 @@ SECTIONS
              *(.hash)
      }
 
-     __rela_offset   = ADDR(.rela) - KIMAGE_VADDR;
+     __rela_offset   = ABSOLUTE(ADDR(.rela) - KIMAGE_VADDR);
      __rela_size     = SIZEOF(.rela);
-     __dynsym_offset = ADDR(.dynsym) - KIMAGE_VADDR;
+     __dynsym_offset = ABSOLUTE(ADDR(.dynsym) - KIMAGE_VADDR);
You removed the __dynsym_offset entry entirely in "arm64: relocatable:
suppress R_AARCH64_ABS64 relocations in vmlinux", so I'll drop this part of the patch and assume you'll do a separate backport for stable. Ok?

Quoted 4 lines

Quoted 2 lines
If you are ok with fixing up patches manually while applying, could we just reorder them instead?
Actually, this should go into v4.8-late not -stable

Then the fixes tag is pretty confusing, since that's a commit from v4.7.

Will

Ard Biesheuvel

author

2016-07-29 10:52 UTC #5 lore raw

PATCH 1/1 arm64: vmlinux.lds: make __rela_offset and __dynsym_offset ABSOLUTE

Quoted 40 lines · Will Deacon

Quoted 39 lines · Ard Biesheuvel
Quoted 38 lines · Ard Biesheuvel
Quoted 36 lines · Will Deacon
Hi Ard,
Quoted 20 lines · Ard Biesheuvel
Due to the untyped KIMAGE_VADDR constant, the linker may not notice that the __rela_offset and __dynsym_offset expressions are absolute values (i.e., are not subject to relocation). This does not matter for KASLR, but it does confuse kallsyms in relative mode, since it uses the lowest non-absolute symbol address as the anchor point, and expects all other symbol addresses to be within 4 GB of it.
Fix this by qualifying these expressions as ABSOLUTE() explicitly.
Fixes 0cd3defe0af4 ("arm64: kernel: perform relocation processing from ID map")
Signed-off-by Ard Biesheuvel <[email protected]>
---
 arch/arm64/kernel/vmlinux.lds.S | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
arch/arm64/kernel/vmlinux.lds.S +0 -0
index 89d6e177ecbd..d49492179967 100644
arch/arm64/kernel/vmlinux.lds.S SECTIONS · line 189
Quoted 8 lines
arch/arm64/kernel/vmlinux.lds.S +2 -2
@@ -184,9 +184,9 @@ SECTIONS
              *(.hash)
      }
 
-     __rela_offset   = ADDR(.rela) - KIMAGE_VADDR;
+     __rela_offset   = ABSOLUTE(ADDR(.rela) - KIMAGE_VADDR);
      __rela_size     = SIZEOF(.rela);
-     __dynsym_offset = ADDR(.dynsym) - KIMAGE_VADDR;
+     __dynsym_offset = ABSOLUTE(ADDR(.dynsym) - KIMAGE_VADDR);
You removed the __dynsym_offset entry entirely in "arm64: relocatable:
suppress R_AARCH64_ABS64 relocations in vmlinux", so I'll drop this part of the patch and assume you'll do a separate backport for stable. Ok?

Quoted 7 lines

Quoted 4 lines
Quoted 2 lines
If you are ok with fixing up patches manually while applying, could we just reorder them instead?
Actually, this should go into v4.8-late not -stable
Then the fixes tag is pretty confusing, since that's a commit from v4.7.

Sorry, my bad. I managed to confuse myself here.
The tag is accurate, I was wrong. This should go into v4.7 as well.

Note that the breakage I experienced was only with an LD built from binutils-gdb HEAD (2.28), so I guess there is no urgency here.

Will Deacon

2016-07-29 10:57 UTC #6 lore raw

PATCH 1/1 arm64: vmlinux.lds: make __rela_offset and __dynsym_offset ABSOLUTE

Quoted 44 lines · Ard Biesheuvel

Quoted 43 lines · Will Deacon
Quoted 42 lines · Ard Biesheuvel
Quoted 40 lines · Ard Biesheuvel
Quoted 26 lines · Will Deacon
Hi Ard,
Quoted 23 lines · Ard Biesheuvel
Due to the untyped KIMAGE_VADDR constant, the linker may not notice that the __rela_offset and __dynsym_offset expressions are absolute values (i.e., are not subject to relocation). This does not matter for KASLR, but it does confuse kallsyms in relative mode, since it uses the lowest non-absolute symbol address as the anchor point, and expects all other symbol addresses to be within 4 GB of it.
Fix this by qualifying these expressions as ABSOLUTE() explicitly.
Fixes 0cd3defe0af4 ("arm64: kernel: perform relocation processing from ID map")
Signed-off-by Ard Biesheuvel <[email protected]>
---
 arch/arm64/kernel/vmlinux.lds.S | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
arch/arm64/kernel/vmlinux.lds.S +0 -0
index 89d6e177ecbd..d49492179967 100644
@@ -184,9 +184,9 @@ SECTIONS
              *(.hash)
      }
 
Quoted 10 lines
arch/arm64/kernel/vmlinux.lds.S SECTIONS · line 189
Quoted 5 lines
arch/arm64/kernel/vmlinux.lds.S +2 -2
-     __rela_offset   = ADDR(.rela) - KIMAGE_VADDR;
+     __rela_offset   = ABSOLUTE(ADDR(.rela) - KIMAGE_VADDR);
      __rela_size     = SIZEOF(.rela);
-     __dynsym_offset = ADDR(.dynsym) - KIMAGE_VADDR;
+     __dynsym_offset = ABSOLUTE(ADDR(.dynsym) - KIMAGE_VADDR);
You removed the __dynsym_offset entry entirely in "arm64: relocatable:
suppress R_AARCH64_ABS64 relocations in vmlinux", so I'll drop this part of the patch and assume you'll do a separate backport for stable. Ok?
If you are ok with fixing up patches manually while applying, could we just reorder them instead?

Quoted 7 lines

Quoted 4 lines
Quoted 1 line
Actually, this should go into v4.8-late not -stable
Then the fixes tag is pretty confusing, since that's a commit from v4.7.
Sorry, my bad. I managed to confuse myself here.
The tag is accurate, I was wrong. This should go into v4.7 as well.

Haha, ok! So if this is v4.7 material, what *should* the fixes tag be?

Quoted 2 lines

Note that the breakage I experienced was only with an LD built from binutils-gdb HEAD (2.28), so I guess there is no urgency here.

Sure, but people might upgrade their binutils before their kernel Image, so the backport is worthwhile.

Will

Ard Biesheuvel

author

2016-07-29 11:01 UTC #7 lore raw

PATCH 1/1 arm64: vmlinux.lds: make __rela_offset and __dynsym_offset ABSOLUTE

Quoted 47 lines · Will Deacon

Quoted 46 lines · Ard Biesheuvel
Quoted 45 lines · Will Deacon
Quoted 43 lines · Ard Biesheuvel
Quoted 38 lines · Ard Biesheuvel
Quoted 26 lines · Will Deacon
Hi Ard,
Quoted 23 lines · Ard Biesheuvel
Due to the untyped KIMAGE_VADDR constant, the linker may not notice that the __rela_offset and __dynsym_offset expressions are absolute values (i.e., are not subject to relocation). This does not matter for KASLR, but it does confuse kallsyms in relative mode, since it uses the lowest non-absolute symbol address as the anchor point, and expects all other symbol addresses to be within 4 GB of it.
Fix this by qualifying these expressions as ABSOLUTE() explicitly.
Fixes 0cd3defe0af4 ("arm64: kernel: perform relocation processing from ID map")
Signed-off-by Ard Biesheuvel <[email protected]>
---
 arch/arm64/kernel/vmlinux.lds.S | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
arch/arm64/kernel/vmlinux.lds.S +0 -0
index 89d6e177ecbd..d49492179967 100644
@@ -184,9 +184,9 @@ SECTIONS
              *(.hash)
      }
 
Quoted 10 lines
arch/arm64/kernel/vmlinux.lds.S SECTIONS · line 189
Quoted 5 lines
arch/arm64/kernel/vmlinux.lds.S +2 -2
-     __rela_offset   = ADDR(.rela) - KIMAGE_VADDR;
+     __rela_offset   = ABSOLUTE(ADDR(.rela) - KIMAGE_VADDR);
      __rela_size     = SIZEOF(.rela);
-     __dynsym_offset = ADDR(.dynsym) - KIMAGE_VADDR;
+     __dynsym_offset = ABSOLUTE(ADDR(.dynsym) - KIMAGE_VADDR);
You removed the __dynsym_offset entry entirely in "arm64: relocatable:
suppress R_AARCH64_ABS64 relocations in vmlinux", so I'll drop this part of the patch and assume you'll do a separate backport for stable. Ok?
Quoted 2 lines
If you are ok with fixing up patches manually while applying, could we just reorder them instead?
Actually, this should go into v4.8-late not -stable

Quoted 8 lines

Quoted 5 lines
Quoted 2 lines
Then the fixes tag is pretty confusing, since that's a commit from v4.7.
Sorry, my bad. I managed to confuse myself here.
The tag is accurate, I was wrong. This should go into v4.7 as well.
Haha, ok! So if this is v4.7 material, what *should* the fixes tag be?

As I said, the fixes tag is accurate. So it should still go into 4.8-late but with a cc:stable. The [tweaked] "suppress R_AARCH64_ABS64 relocations in vmlinux" could simply go on top, with no backport (it is just an optimization, not a bugfix)

Quoted 6 lines

Quoted 2 lines
Note that the breakage I experienced was only with an LD built from binutils-gdb HEAD (2.28), so I guess there is no urgency here.
Sure, but people might upgrade their binutils before their kernel Image, so the backport is worthwhile.

That was my reasoning as well, but I was just highlighting that nothing is broken currently with released toolchains.