* [PATCH] ARM: Fix zImage file size not aligned with CONFIG_EFI_STUB enabled
@ 2017-10-18 5:01 ` Jeffy Chen
0 siblings, 0 replies; 42+ messages in thread
From: Jeffy Chen @ 2017-10-18 5:01 UTC (permalink / raw)
To: linux-arm-kernel
The zImage file size should be aligned.
Fixes: e4bae4d0b5f3 ("arm/efi: Split zImage code and data into separate PE/COFF sections")
Signed-off-by: Jeffy Chen <jeffy.chen@rock-chips.com>
---
arch/arm/boot/compressed/vmlinux.lds.S | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/arch/arm/boot/compressed/vmlinux.lds.S b/arch/arm/boot/compressed/vmlinux.lds.S
index b38dcef90756..1636fa259577 100644
--- a/arch/arm/boot/compressed/vmlinux.lds.S
+++ b/arch/arm/boot/compressed/vmlinux.lds.S
@@ -70,10 +70,6 @@ SECTIONS
.got : { *(.got) }
_got_end = .;
- /* ensure the zImage file size is always a multiple of 64 bits */
- /* (without a dummy byte, ld just ignores the empty section) */
- .pad : { BYTE(0); . = ALIGN(8); }
-
#ifdef CONFIG_EFI_STUB
.data : ALIGN(4096) {
__pecoff_data_start = .;
@@ -93,6 +89,10 @@ SECTIONS
__pecoff_data_rawsize = . - ADDR(.data);
#endif
+ /* ensure the zImage file size is always a multiple of 64 bits */
+ /* (without a dummy byte, ld just ignores the empty section) */
+ .pad : { BYTE(0); . = ALIGN(8); }
+
_edata = .;
_magic_sig = ZIMAGE_MAGIC(0x016f2818);
--
2.11.0
^ permalink raw reply related [flat|nested] 42+ messages in thread* [PATCH] ARM: Fix zImage file size not aligned with CONFIG_EFI_STUB enabled @ 2017-10-18 5:01 ` Jeffy Chen 0 siblings, 0 replies; 42+ messages in thread From: Jeffy Chen @ 2017-10-18 5:01 UTC (permalink / raw) To: linux-kernel Cc: chris.zhong, Jeffy Chen, Ingo Molnar, Ard Biesheuvel, Russell King, linux-arm-kernel The zImage file size should be aligned. Fixes: e4bae4d0b5f3 ("arm/efi: Split zImage code and data into separate PE/COFF sections") Signed-off-by: Jeffy Chen <jeffy.chen@rock-chips.com> --- arch/arm/boot/compressed/vmlinux.lds.S | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/arch/arm/boot/compressed/vmlinux.lds.S b/arch/arm/boot/compressed/vmlinux.lds.S index b38dcef90756..1636fa259577 100644 --- a/arch/arm/boot/compressed/vmlinux.lds.S +++ b/arch/arm/boot/compressed/vmlinux.lds.S @@ -70,10 +70,6 @@ SECTIONS .got : { *(.got) } _got_end = .; - /* ensure the zImage file size is always a multiple of 64 bits */ - /* (without a dummy byte, ld just ignores the empty section) */ - .pad : { BYTE(0); . = ALIGN(8); } - #ifdef CONFIG_EFI_STUB .data : ALIGN(4096) { __pecoff_data_start = .; @@ -93,6 +89,10 @@ SECTIONS __pecoff_data_rawsize = . - ADDR(.data); #endif + /* ensure the zImage file size is always a multiple of 64 bits */ + /* (without a dummy byte, ld just ignores the empty section) */ + .pad : { BYTE(0); . = ALIGN(8); } + _edata = .; _magic_sig = ZIMAGE_MAGIC(0x016f2818); -- 2.11.0 ^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH] ARM: Fix zImage file size not aligned with CONFIG_EFI_STUB enabled 2017-10-18 5:01 ` Jeffy Chen @ 2017-10-18 6:19 ` Chris Zhong -1 siblings, 0 replies; 42+ messages in thread From: Chris Zhong @ 2017-10-18 6:19 UTC (permalink / raw) To: linux-arm-kernel Tested-by: Chris Zhong <zyw@rock-chips.com> On Wednesday, October 18, 2017 01:01 PM, Jeffy Chen wrote: > The zImage file size should be aligned. > > Fixes: e4bae4d0b5f3 ("arm/efi: Split zImage code and data into separate PE/COFF sections") > Signed-off-by: Jeffy Chen <jeffy.chen@rock-chips.com> > --- > > arch/arm/boot/compressed/vmlinux.lds.S | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/arch/arm/boot/compressed/vmlinux.lds.S b/arch/arm/boot/compressed/vmlinux.lds.S > index b38dcef90756..1636fa259577 100644 > --- a/arch/arm/boot/compressed/vmlinux.lds.S > +++ b/arch/arm/boot/compressed/vmlinux.lds.S > @@ -70,10 +70,6 @@ SECTIONS > .got : { *(.got) } > _got_end = .; > > - /* ensure the zImage file size is always a multiple of 64 bits */ > - /* (without a dummy byte, ld just ignores the empty section) */ > - .pad : { BYTE(0); . = ALIGN(8); } > - > #ifdef CONFIG_EFI_STUB > .data : ALIGN(4096) { > __pecoff_data_start = .; > @@ -93,6 +89,10 @@ SECTIONS > __pecoff_data_rawsize = . - ADDR(.data); > #endif > > + /* ensure the zImage file size is always a multiple of 64 bits */ > + /* (without a dummy byte, ld just ignores the empty section) */ > + .pad : { BYTE(0); . = ALIGN(8); } > + > _edata = .; > > _magic_sig = ZIMAGE_MAGIC(0x016f2818); -- Chris Zhong ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH] ARM: Fix zImage file size not aligned with CONFIG_EFI_STUB enabled @ 2017-10-18 6:19 ` Chris Zhong 0 siblings, 0 replies; 42+ messages in thread From: Chris Zhong @ 2017-10-18 6:19 UTC (permalink / raw) To: Jeffy Chen, linux-kernel Cc: chris.zhong, Ingo Molnar, Ard Biesheuvel, Russell King, linux-arm-kernel Tested-by: Chris Zhong <zyw@rock-chips.com> On Wednesday, October 18, 2017 01:01 PM, Jeffy Chen wrote: > The zImage file size should be aligned. > > Fixes: e4bae4d0b5f3 ("arm/efi: Split zImage code and data into separate PE/COFF sections") > Signed-off-by: Jeffy Chen <jeffy.chen@rock-chips.com> > --- > > arch/arm/boot/compressed/vmlinux.lds.S | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/arch/arm/boot/compressed/vmlinux.lds.S b/arch/arm/boot/compressed/vmlinux.lds.S > index b38dcef90756..1636fa259577 100644 > --- a/arch/arm/boot/compressed/vmlinux.lds.S > +++ b/arch/arm/boot/compressed/vmlinux.lds.S > @@ -70,10 +70,6 @@ SECTIONS > .got : { *(.got) } > _got_end = .; > > - /* ensure the zImage file size is always a multiple of 64 bits */ > - /* (without a dummy byte, ld just ignores the empty section) */ > - .pad : { BYTE(0); . = ALIGN(8); } > - > #ifdef CONFIG_EFI_STUB > .data : ALIGN(4096) { > __pecoff_data_start = .; > @@ -93,6 +89,10 @@ SECTIONS > __pecoff_data_rawsize = . - ADDR(.data); > #endif > > + /* ensure the zImage file size is always a multiple of 64 bits */ > + /* (without a dummy byte, ld just ignores the empty section) */ > + .pad : { BYTE(0); . = ALIGN(8); } > + > _edata = .; > > _magic_sig = ZIMAGE_MAGIC(0x016f2818); -- Chris Zhong ^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH] ARM: Fix zImage file size not aligned with CONFIG_EFI_STUB enabled 2017-10-18 5:01 ` Jeffy Chen @ 2017-10-22 11:01 ` Ard Biesheuvel -1 siblings, 0 replies; 42+ messages in thread From: Ard Biesheuvel @ 2017-10-22 11:01 UTC (permalink / raw) To: linux-arm-kernel On 18 October 2017 at 06:01, Jeffy Chen <jeffy.chen@rock-chips.com> wrote: > The zImage file size should be aligned. > > Fixes: e4bae4d0b5f3 ("arm/efi: Split zImage code and data into separate PE/COFF sections") > Signed-off-by: Jeffy Chen <jeffy.chen@rock-chips.com> > --- > > arch/arm/boot/compressed/vmlinux.lds.S | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/arch/arm/boot/compressed/vmlinux.lds.S b/arch/arm/boot/compressed/vmlinux.lds.S > index b38dcef90756..1636fa259577 100644 > --- a/arch/arm/boot/compressed/vmlinux.lds.S > +++ b/arch/arm/boot/compressed/vmlinux.lds.S > @@ -70,10 +70,6 @@ SECTIONS > .got : { *(.got) } > _got_end = .; > > - /* ensure the zImage file size is always a multiple of 64 bits */ > - /* (without a dummy byte, ld just ignores the empty section) */ > - .pad : { BYTE(0); . = ALIGN(8); } > - > #ifdef CONFIG_EFI_STUB > .data : ALIGN(4096) { > __pecoff_data_start = .; > @@ -93,6 +89,10 @@ SECTIONS > __pecoff_data_rawsize = . - ADDR(.data); > #endif > > + /* ensure the zImage file size is always a multiple of 64 bits */ > + /* (without a dummy byte, ld just ignores the empty section) */ > + .pad : { BYTE(0); . = ALIGN(8); } > + > _edata = .; > > _magic_sig = ZIMAGE_MAGIC(0x016f2818); > -- > 2.11.0 > This is not the right fix. If CONFIG_EFI_STUB is enabled, the zImage filesize should be rounded up to 512 bytes not 8 bytes. The '. = ALIGN(512);' in the .data section appears to ensure that, but for some reason, that appears not to be working. Could you share the output of $ readelf -S arch/arm/boot/compressed/vmlinux please? And could you please check whether this patch https://marc.info/?l=linux-arm-kernel&m=150488477807353 fixes the issue for you? Thanks, Ard. ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH] ARM: Fix zImage file size not aligned with CONFIG_EFI_STUB enabled @ 2017-10-22 11:01 ` Ard Biesheuvel 0 siblings, 0 replies; 42+ messages in thread From: Ard Biesheuvel @ 2017-10-22 11:01 UTC (permalink / raw) To: Jeffy Chen, Russell King Cc: linux-kernel@vger.kernel.org, chris.zhong, Ingo Molnar, linux-arm-kernel@lists.infradead.org On 18 October 2017 at 06:01, Jeffy Chen <jeffy.chen@rock-chips.com> wrote: > The zImage file size should be aligned. > > Fixes: e4bae4d0b5f3 ("arm/efi: Split zImage code and data into separate PE/COFF sections") > Signed-off-by: Jeffy Chen <jeffy.chen@rock-chips.com> > --- > > arch/arm/boot/compressed/vmlinux.lds.S | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/arch/arm/boot/compressed/vmlinux.lds.S b/arch/arm/boot/compressed/vmlinux.lds.S > index b38dcef90756..1636fa259577 100644 > --- a/arch/arm/boot/compressed/vmlinux.lds.S > +++ b/arch/arm/boot/compressed/vmlinux.lds.S > @@ -70,10 +70,6 @@ SECTIONS > .got : { *(.got) } > _got_end = .; > > - /* ensure the zImage file size is always a multiple of 64 bits */ > - /* (without a dummy byte, ld just ignores the empty section) */ > - .pad : { BYTE(0); . = ALIGN(8); } > - > #ifdef CONFIG_EFI_STUB > .data : ALIGN(4096) { > __pecoff_data_start = .; > @@ -93,6 +89,10 @@ SECTIONS > __pecoff_data_rawsize = . - ADDR(.data); > #endif > > + /* ensure the zImage file size is always a multiple of 64 bits */ > + /* (without a dummy byte, ld just ignores the empty section) */ > + .pad : { BYTE(0); . = ALIGN(8); } > + > _edata = .; > > _magic_sig = ZIMAGE_MAGIC(0x016f2818); > -- > 2.11.0 > This is not the right fix. If CONFIG_EFI_STUB is enabled, the zImage filesize should be rounded up to 512 bytes not 8 bytes. The '. = ALIGN(512);' in the .data section appears to ensure that, but for some reason, that appears not to be working. Could you share the output of $ readelf -S arch/arm/boot/compressed/vmlinux please? And could you please check whether this patch https://marc.info/?l=linux-arm-kernel&m=150488477807353 fixes the issue for you? Thanks, Ard. ^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH] ARM: Fix zImage file size not aligned with CONFIG_EFI_STUB enabled 2017-10-22 11:01 ` Ard Biesheuvel @ 2017-10-22 12:47 ` Russell King - ARM Linux -1 siblings, 0 replies; 42+ messages in thread From: Russell King - ARM Linux @ 2017-10-22 12:47 UTC (permalink / raw) To: linux-arm-kernel On Sun, Oct 22, 2017 at 12:01:13PM +0100, Ard Biesheuvel wrote: > On 18 October 2017 at 06:01, Jeffy Chen <jeffy.chen@rock-chips.com> wrote: > > The zImage file size should be aligned. > > > > Fixes: e4bae4d0b5f3 ("arm/efi: Split zImage code and data into separate PE/COFF sections") > > Signed-off-by: Jeffy Chen <jeffy.chen@rock-chips.com> > > --- > > > > arch/arm/boot/compressed/vmlinux.lds.S | 8 ++++---- > > 1 file changed, 4 insertions(+), 4 deletions(-) > > > > diff --git a/arch/arm/boot/compressed/vmlinux.lds.S b/arch/arm/boot/compressed/vmlinux.lds.S > > index b38dcef90756..1636fa259577 100644 > > --- a/arch/arm/boot/compressed/vmlinux.lds.S > > +++ b/arch/arm/boot/compressed/vmlinux.lds.S > > @@ -70,10 +70,6 @@ SECTIONS > > .got : { *(.got) } > > _got_end = .; > > > > - /* ensure the zImage file size is always a multiple of 64 bits */ > > - /* (without a dummy byte, ld just ignores the empty section) */ > > - .pad : { BYTE(0); . = ALIGN(8); } > > - > > #ifdef CONFIG_EFI_STUB > > .data : ALIGN(4096) { > > __pecoff_data_start = .; > > @@ -93,6 +89,10 @@ SECTIONS > > __pecoff_data_rawsize = . - ADDR(.data); > > #endif > > > > + /* ensure the zImage file size is always a multiple of 64 bits */ > > + /* (without a dummy byte, ld just ignores the empty section) */ > > + .pad : { BYTE(0); . = ALIGN(8); } > > + > > _edata = .; > > > > _magic_sig = ZIMAGE_MAGIC(0x016f2818); > > -- > > 2.11.0 > > > > This is not the right fix. If CONFIG_EFI_STUB is enabled, the zImage > filesize should be rounded up to 512 bytes not 8 bytes. The '. = > ALIGN(512);' in the .data section appears to ensure that, but for some > reason, that appears not to be working. Actually, the existing .pad section is totally and utterly bogus when EFI is enabled: . = ALIGN(4); _etext = .; .got.plt : { *(.got.plt) } _got_start = .; .got : { *(.got) } _got_end = .; The .got.plt and .got are always word-based. This is then followed by .pad, which does nothing but pad out to a multiple of 64 bit: /* ensure the zImage file size is always a multiple of 64 bits */ /* (without a dummy byte, ld just ignores the empty section) */ .pad : { BYTE(0); . = ALIGN(8); } So this may add zero or 4 bytes of padding. This is then followed by the EFI data: .data : ALIGN(4096) { ... . = ALIGN(512); } which is aligned to 4K but aligns the end of itself to 512. So, we have the end of .got aligned to 4, followed by .pad that tries to align to 8, followed by an optional .data section. This is pointless. A sane patch would be to choose between the EFI .data section and the .pad section. So, it should be: #ifdef CONFIG_EFI_STUB .data : ALIGN(4096) { ... . = ALIGN(512); } #else .pad : { BYTE(0); . = ALIGN(8); } #endif -- RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up According to speedtest.net: 8.21Mbps down 510kbps up ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH] ARM: Fix zImage file size not aligned with CONFIG_EFI_STUB enabled @ 2017-10-22 12:47 ` Russell King - ARM Linux 0 siblings, 0 replies; 42+ messages in thread From: Russell King - ARM Linux @ 2017-10-22 12:47 UTC (permalink / raw) To: Ard Biesheuvel Cc: Jeffy Chen, linux-kernel@vger.kernel.org, chris.zhong, Ingo Molnar, linux-arm-kernel@lists.infradead.org On Sun, Oct 22, 2017 at 12:01:13PM +0100, Ard Biesheuvel wrote: > On 18 October 2017 at 06:01, Jeffy Chen <jeffy.chen@rock-chips.com> wrote: > > The zImage file size should be aligned. > > > > Fixes: e4bae4d0b5f3 ("arm/efi: Split zImage code and data into separate PE/COFF sections") > > Signed-off-by: Jeffy Chen <jeffy.chen@rock-chips.com> > > --- > > > > arch/arm/boot/compressed/vmlinux.lds.S | 8 ++++---- > > 1 file changed, 4 insertions(+), 4 deletions(-) > > > > diff --git a/arch/arm/boot/compressed/vmlinux.lds.S b/arch/arm/boot/compressed/vmlinux.lds.S > > index b38dcef90756..1636fa259577 100644 > > --- a/arch/arm/boot/compressed/vmlinux.lds.S > > +++ b/arch/arm/boot/compressed/vmlinux.lds.S > > @@ -70,10 +70,6 @@ SECTIONS > > .got : { *(.got) } > > _got_end = .; > > > > - /* ensure the zImage file size is always a multiple of 64 bits */ > > - /* (without a dummy byte, ld just ignores the empty section) */ > > - .pad : { BYTE(0); . = ALIGN(8); } > > - > > #ifdef CONFIG_EFI_STUB > > .data : ALIGN(4096) { > > __pecoff_data_start = .; > > @@ -93,6 +89,10 @@ SECTIONS > > __pecoff_data_rawsize = . - ADDR(.data); > > #endif > > > > + /* ensure the zImage file size is always a multiple of 64 bits */ > > + /* (without a dummy byte, ld just ignores the empty section) */ > > + .pad : { BYTE(0); . = ALIGN(8); } > > + > > _edata = .; > > > > _magic_sig = ZIMAGE_MAGIC(0x016f2818); > > -- > > 2.11.0 > > > > This is not the right fix. If CONFIG_EFI_STUB is enabled, the zImage > filesize should be rounded up to 512 bytes not 8 bytes. The '. = > ALIGN(512);' in the .data section appears to ensure that, but for some > reason, that appears not to be working. Actually, the existing .pad section is totally and utterly bogus when EFI is enabled: . = ALIGN(4); _etext = .; .got.plt : { *(.got.plt) } _got_start = .; .got : { *(.got) } _got_end = .; The .got.plt and .got are always word-based. This is then followed by .pad, which does nothing but pad out to a multiple of 64 bit: /* ensure the zImage file size is always a multiple of 64 bits */ /* (without a dummy byte, ld just ignores the empty section) */ .pad : { BYTE(0); . = ALIGN(8); } So this may add zero or 4 bytes of padding. This is then followed by the EFI data: .data : ALIGN(4096) { ... . = ALIGN(512); } which is aligned to 4K but aligns the end of itself to 512. So, we have the end of .got aligned to 4, followed by .pad that tries to align to 8, followed by an optional .data section. This is pointless. A sane patch would be to choose between the EFI .data section and the .pad section. So, it should be: #ifdef CONFIG_EFI_STUB .data : ALIGN(4096) { ... . = ALIGN(512); } #else .pad : { BYTE(0); . = ALIGN(8); } #endif -- RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up According to speedtest.net: 8.21Mbps down 510kbps up ^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH] ARM: Fix zImage file size not aligned with CONFIG_EFI_STUB enabled 2017-10-22 12:47 ` Russell King - ARM Linux @ 2017-10-22 13:01 ` Ard Biesheuvel -1 siblings, 0 replies; 42+ messages in thread From: Ard Biesheuvel @ 2017-10-22 13:01 UTC (permalink / raw) To: linux-arm-kernel On 22 October 2017 at 13:47, Russell King - ARM Linux <linux@armlinux.org.uk> wrote: > On Sun, Oct 22, 2017 at 12:01:13PM +0100, Ard Biesheuvel wrote: >> On 18 October 2017 at 06:01, Jeffy Chen <jeffy.chen@rock-chips.com> wrote: >> > The zImage file size should be aligned. >> > >> > Fixes: e4bae4d0b5f3 ("arm/efi: Split zImage code and data into separate PE/COFF sections") >> > Signed-off-by: Jeffy Chen <jeffy.chen@rock-chips.com> >> > --- >> > >> > arch/arm/boot/compressed/vmlinux.lds.S | 8 ++++---- >> > 1 file changed, 4 insertions(+), 4 deletions(-) >> > >> > diff --git a/arch/arm/boot/compressed/vmlinux.lds.S b/arch/arm/boot/compressed/vmlinux.lds.S >> > index b38dcef90756..1636fa259577 100644 >> > --- a/arch/arm/boot/compressed/vmlinux.lds.S >> > +++ b/arch/arm/boot/compressed/vmlinux.lds.S >> > @@ -70,10 +70,6 @@ SECTIONS >> > .got : { *(.got) } >> > _got_end = .; >> > >> > - /* ensure the zImage file size is always a multiple of 64 bits */ >> > - /* (without a dummy byte, ld just ignores the empty section) */ >> > - .pad : { BYTE(0); . = ALIGN(8); } >> > - >> > #ifdef CONFIG_EFI_STUB >> > .data : ALIGN(4096) { >> > __pecoff_data_start = .; >> > @@ -93,6 +89,10 @@ SECTIONS >> > __pecoff_data_rawsize = . - ADDR(.data); >> > #endif >> > >> > + /* ensure the zImage file size is always a multiple of 64 bits */ >> > + /* (without a dummy byte, ld just ignores the empty section) */ >> > + .pad : { BYTE(0); . = ALIGN(8); } >> > + >> > _edata = .; >> > >> > _magic_sig = ZIMAGE_MAGIC(0x016f2818); >> > -- >> > 2.11.0 >> > >> >> This is not the right fix. If CONFIG_EFI_STUB is enabled, the zImage >> filesize should be rounded up to 512 bytes not 8 bytes. The '. = >> ALIGN(512);' in the .data section appears to ensure that, but for some >> reason, that appears not to be working. > > Actually, the existing .pad section is totally and utterly bogus when > EFI is enabled: > > . = ALIGN(4); > _etext = .; > > .got.plt : { *(.got.plt) } > _got_start = .; > .got : { *(.got) } > _got_end = .; > > The .got.plt and .got are always word-based. This is then followed by > .pad, which does nothing but pad out to a multiple of 64 bit: > > /* ensure the zImage file size is always a multiple of 64 bits */ > /* (without a dummy byte, ld just ignores the empty section) */ > .pad : { BYTE(0); . = ALIGN(8); } > > So this may add zero or 4 bytes of padding. > > This is then followed by the EFI data: > > .data : ALIGN(4096) { > ... > . = ALIGN(512); > } > > which is aligned to 4K but aligns the end of itself to 512. > > So, we have the end of .got aligned to 4, followed by .pad that tries to > align to 8, followed by an optional .data section. This is pointless. > > A sane patch would be to choose between the EFI .data section and the > .pad section. So, it should be: > > #ifdef CONFIG_EFI_STUB > .data : ALIGN(4096) { > ... > . = ALIGN(512); > } > #else > .pad : { BYTE(0); . = ALIGN(8); } > #endif > Agreed, the .pad section has no point for EFI_STUB=y. However, it seems this symptom is caused by the same issues I am trying to address here https://marc.info/?l=linux-arm-kernel&m=150488477807353 which is that we have __ksymtab_xxx sections that we should discard, because the linker will otherwise emit them /after/ .data or .pad. This is caused by the use of lib/sort.c in the EFI stub, which contains an EXPORT_SYMBOL(). Would you perhaps prefer that I clone sort.c into its own .c file specifically for the EFI stub? (under drivers/firmware/efi/libstub) That should get rid of these spurious sections and thus the misalignments and/or movements that are causing all of these issues. ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH] ARM: Fix zImage file size not aligned with CONFIG_EFI_STUB enabled @ 2017-10-22 13:01 ` Ard Biesheuvel 0 siblings, 0 replies; 42+ messages in thread From: Ard Biesheuvel @ 2017-10-22 13:01 UTC (permalink / raw) To: Russell King - ARM Linux Cc: Jeffy Chen, linux-kernel@vger.kernel.org, chris.zhong, Ingo Molnar, linux-arm-kernel@lists.infradead.org On 22 October 2017 at 13:47, Russell King - ARM Linux <linux@armlinux.org.uk> wrote: > On Sun, Oct 22, 2017 at 12:01:13PM +0100, Ard Biesheuvel wrote: >> On 18 October 2017 at 06:01, Jeffy Chen <jeffy.chen@rock-chips.com> wrote: >> > The zImage file size should be aligned. >> > >> > Fixes: e4bae4d0b5f3 ("arm/efi: Split zImage code and data into separate PE/COFF sections") >> > Signed-off-by: Jeffy Chen <jeffy.chen@rock-chips.com> >> > --- >> > >> > arch/arm/boot/compressed/vmlinux.lds.S | 8 ++++---- >> > 1 file changed, 4 insertions(+), 4 deletions(-) >> > >> > diff --git a/arch/arm/boot/compressed/vmlinux.lds.S b/arch/arm/boot/compressed/vmlinux.lds.S >> > index b38dcef90756..1636fa259577 100644 >> > --- a/arch/arm/boot/compressed/vmlinux.lds.S >> > +++ b/arch/arm/boot/compressed/vmlinux.lds.S >> > @@ -70,10 +70,6 @@ SECTIONS >> > .got : { *(.got) } >> > _got_end = .; >> > >> > - /* ensure the zImage file size is always a multiple of 64 bits */ >> > - /* (without a dummy byte, ld just ignores the empty section) */ >> > - .pad : { BYTE(0); . = ALIGN(8); } >> > - >> > #ifdef CONFIG_EFI_STUB >> > .data : ALIGN(4096) { >> > __pecoff_data_start = .; >> > @@ -93,6 +89,10 @@ SECTIONS >> > __pecoff_data_rawsize = . - ADDR(.data); >> > #endif >> > >> > + /* ensure the zImage file size is always a multiple of 64 bits */ >> > + /* (without a dummy byte, ld just ignores the empty section) */ >> > + .pad : { BYTE(0); . = ALIGN(8); } >> > + >> > _edata = .; >> > >> > _magic_sig = ZIMAGE_MAGIC(0x016f2818); >> > -- >> > 2.11.0 >> > >> >> This is not the right fix. If CONFIG_EFI_STUB is enabled, the zImage >> filesize should be rounded up to 512 bytes not 8 bytes. The '. = >> ALIGN(512);' in the .data section appears to ensure that, but for some >> reason, that appears not to be working. > > Actually, the existing .pad section is totally and utterly bogus when > EFI is enabled: > > . = ALIGN(4); > _etext = .; > > .got.plt : { *(.got.plt) } > _got_start = .; > .got : { *(.got) } > _got_end = .; > > The .got.plt and .got are always word-based. This is then followed by > .pad, which does nothing but pad out to a multiple of 64 bit: > > /* ensure the zImage file size is always a multiple of 64 bits */ > /* (without a dummy byte, ld just ignores the empty section) */ > .pad : { BYTE(0); . = ALIGN(8); } > > So this may add zero or 4 bytes of padding. > > This is then followed by the EFI data: > > .data : ALIGN(4096) { > ... > . = ALIGN(512); > } > > which is aligned to 4K but aligns the end of itself to 512. > > So, we have the end of .got aligned to 4, followed by .pad that tries to > align to 8, followed by an optional .data section. This is pointless. > > A sane patch would be to choose between the EFI .data section and the > .pad section. So, it should be: > > #ifdef CONFIG_EFI_STUB > .data : ALIGN(4096) { > ... > . = ALIGN(512); > } > #else > .pad : { BYTE(0); . = ALIGN(8); } > #endif > Agreed, the .pad section has no point for EFI_STUB=y. However, it seems this symptom is caused by the same issues I am trying to address here https://marc.info/?l=linux-arm-kernel&m=150488477807353 which is that we have __ksymtab_xxx sections that we should discard, because the linker will otherwise emit them /after/ .data or .pad. This is caused by the use of lib/sort.c in the EFI stub, which contains an EXPORT_SYMBOL(). Would you perhaps prefer that I clone sort.c into its own .c file specifically for the EFI stub? (under drivers/firmware/efi/libstub) That should get rid of these spurious sections and thus the misalignments and/or movements that are causing all of these issues. ^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH] ARM: Fix zImage file size not aligned with CONFIG_EFI_STUB enabled 2017-10-22 13:01 ` Ard Biesheuvel @ 2017-10-23 3:26 ` jeffy -1 siblings, 0 replies; 42+ messages in thread From: jeffy @ 2017-10-23 3:26 UTC (permalink / raw) To: linux-arm-kernel Hi Ard, On 10/22/2017 09:01 PM, Ard Biesheuvel wrote: > On 22 October 2017 at 13:47, Russell King - ARM Linux > <linux@armlinux.org.uk> wrote: >> On Sun, Oct 22, 2017 at 12:01:13PM +0100, Ard Biesheuvel wrote: >>> On 18 October 2017 at 06:01, Jeffy Chen <jeffy.chen@rock-chips.com> wrote: >>>> The zImage file size should be aligned. >>>> >>>> Fixes: e4bae4d0b5f3 ("arm/efi: Split zImage code and data into separate PE/COFF sections") >>>> Signed-off-by: Jeffy Chen <jeffy.chen@rock-chips.com> >>>> --- >>>> >>>> arch/arm/boot/compressed/vmlinux.lds.S | 8 ++++---- >>>> 1 file changed, 4 insertions(+), 4 deletions(-) >>>> >>>> diff --git a/arch/arm/boot/compressed/vmlinux.lds.S b/arch/arm/boot/compressed/vmlinux.lds.S >>>> index b38dcef90756..1636fa259577 100644 >>>> --- a/arch/arm/boot/compressed/vmlinux.lds.S >>>> +++ b/arch/arm/boot/compressed/vmlinux.lds.S >>>> @@ -70,10 +70,6 @@ SECTIONS >>>> .got : { *(.got) } >>>> _got_end = .; >>>> >>>> - /* ensure the zImage file size is always a multiple of 64 bits */ >>>> - /* (without a dummy byte, ld just ignores the empty section) */ >>>> - .pad : { BYTE(0); . = ALIGN(8); } >>>> - >>>> #ifdef CONFIG_EFI_STUB >>>> .data : ALIGN(4096) { >>>> __pecoff_data_start = .; >>>> @@ -93,6 +89,10 @@ SECTIONS >>>> __pecoff_data_rawsize = . - ADDR(.data); >>>> #endif >>>> >>>> + /* ensure the zImage file size is always a multiple of 64 bits */ >>>> + /* (without a dummy byte, ld just ignores the empty section) */ >>>> + .pad : { BYTE(0); . = ALIGN(8); } >>>> + >>>> _edata = .; >>>> >>>> _magic_sig = ZIMAGE_MAGIC(0x016f2818); >>>> -- >>>> 2.11.0 >>>> >>> >>> This is not the right fix. If CONFIG_EFI_STUB is enabled, the zImage >>> filesize should be rounded up to 512 bytes not 8 bytes. The '. = >>> ALIGN(512);' in the .data section appears to ensure that, but for some >>> reason, that appears not to be working. >> >> Actually, the existing .pad section is totally and utterly bogus when >> EFI is enabled: >> >> . = ALIGN(4); >> _etext = .; >> >> .got.plt : { *(.got.plt) } >> _got_start = .; >> .got : { *(.got) } >> _got_end = .; >> >> The .got.plt and .got are always word-based. This is then followed by >> .pad, which does nothing but pad out to a multiple of 64 bit: >> >> /* ensure the zImage file size is always a multiple of 64 bits */ >> /* (without a dummy byte, ld just ignores the empty section) */ >> .pad : { BYTE(0); . = ALIGN(8); } >> >> So this may add zero or 4 bytes of padding. >> >> This is then followed by the EFI data: >> >> .data : ALIGN(4096) { >> ... >> . = ALIGN(512); >> } >> >> which is aligned to 4K but aligns the end of itself to 512. >> >> So, we have the end of .got aligned to 4, followed by .pad that tries to >> align to 8, followed by an optional .data section. This is pointless. >> >> A sane patch would be to choose between the EFI .data section and the >> .pad section. So, it should be: >> >> #ifdef CONFIG_EFI_STUB >> .data : ALIGN(4096) { >> ... >> . = ALIGN(512); >> } >> #else >> .pad : { BYTE(0); . = ALIGN(8); } >> #endif >> > > Agreed, the .pad section has no point for EFI_STUB=y. However, it > seems this symptom is caused by the same issues I am trying to address > here > > https://marc.info/?l=linux-arm-kernel&m=150488477807353 > > which is that we have __ksymtab_xxx sections that we should discard, > because the linker will otherwise emit them /after/ .data or .pad. > This is caused by the use of lib/sort.c in the EFI stub, which > contains an EXPORT_SYMBOL(). hmm, right, didn't notice the data is already aligned... so it's indeed caused by the ksym: [ 9] .data PROGBITS 006ce000 6d6000 000200 00 WA 0 0 4096 [10] ___ksymtab+sort PROGBITS 006ce200 6d6200 000008 00 WA 0 0 4 [11] .bss NOBITS 006ce208 6d6208 00001c 00 WA 0 0 4 and both of your old([PATCH] ARM: compressed: discard ksym/kcrctab input section) and new([PATCH] efi/libstub: arm: omit sorting of the UEFI memory map) patches fix the issue i meet, thanks:) > > Would you perhaps prefer that I clone sort.c into its own .c file > specifically for the EFI stub? (under drivers/firmware/efi/libstub) > That should get rid of these spurious sections and thus the > misalignments and/or movements that are causing all of these issues. > > > ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH] ARM: Fix zImage file size not aligned with CONFIG_EFI_STUB enabled @ 2017-10-23 3:26 ` jeffy 0 siblings, 0 replies; 42+ messages in thread From: jeffy @ 2017-10-23 3:26 UTC (permalink / raw) To: Ard Biesheuvel, Russell King - ARM Linux Cc: linux-kernel@vger.kernel.org, chris.zhong, Ingo Molnar, linux-arm-kernel@lists.infradead.org Hi Ard, On 10/22/2017 09:01 PM, Ard Biesheuvel wrote: > On 22 October 2017 at 13:47, Russell King - ARM Linux > <linux@armlinux.org.uk> wrote: >> On Sun, Oct 22, 2017 at 12:01:13PM +0100, Ard Biesheuvel wrote: >>> On 18 October 2017 at 06:01, Jeffy Chen <jeffy.chen@rock-chips.com> wrote: >>>> The zImage file size should be aligned. >>>> >>>> Fixes: e4bae4d0b5f3 ("arm/efi: Split zImage code and data into separate PE/COFF sections") >>>> Signed-off-by: Jeffy Chen <jeffy.chen@rock-chips.com> >>>> --- >>>> >>>> arch/arm/boot/compressed/vmlinux.lds.S | 8 ++++---- >>>> 1 file changed, 4 insertions(+), 4 deletions(-) >>>> >>>> diff --git a/arch/arm/boot/compressed/vmlinux.lds.S b/arch/arm/boot/compressed/vmlinux.lds.S >>>> index b38dcef90756..1636fa259577 100644 >>>> --- a/arch/arm/boot/compressed/vmlinux.lds.S >>>> +++ b/arch/arm/boot/compressed/vmlinux.lds.S >>>> @@ -70,10 +70,6 @@ SECTIONS >>>> .got : { *(.got) } >>>> _got_end = .; >>>> >>>> - /* ensure the zImage file size is always a multiple of 64 bits */ >>>> - /* (without a dummy byte, ld just ignores the empty section) */ >>>> - .pad : { BYTE(0); . = ALIGN(8); } >>>> - >>>> #ifdef CONFIG_EFI_STUB >>>> .data : ALIGN(4096) { >>>> __pecoff_data_start = .; >>>> @@ -93,6 +89,10 @@ SECTIONS >>>> __pecoff_data_rawsize = . - ADDR(.data); >>>> #endif >>>> >>>> + /* ensure the zImage file size is always a multiple of 64 bits */ >>>> + /* (without a dummy byte, ld just ignores the empty section) */ >>>> + .pad : { BYTE(0); . = ALIGN(8); } >>>> + >>>> _edata = .; >>>> >>>> _magic_sig = ZIMAGE_MAGIC(0x016f2818); >>>> -- >>>> 2.11.0 >>>> >>> >>> This is not the right fix. If CONFIG_EFI_STUB is enabled, the zImage >>> filesize should be rounded up to 512 bytes not 8 bytes. The '. = >>> ALIGN(512);' in the .data section appears to ensure that, but for some >>> reason, that appears not to be working. >> >> Actually, the existing .pad section is totally and utterly bogus when >> EFI is enabled: >> >> . = ALIGN(4); >> _etext = .; >> >> .got.plt : { *(.got.plt) } >> _got_start = .; >> .got : { *(.got) } >> _got_end = .; >> >> The .got.plt and .got are always word-based. This is then followed by >> .pad, which does nothing but pad out to a multiple of 64 bit: >> >> /* ensure the zImage file size is always a multiple of 64 bits */ >> /* (without a dummy byte, ld just ignores the empty section) */ >> .pad : { BYTE(0); . = ALIGN(8); } >> >> So this may add zero or 4 bytes of padding. >> >> This is then followed by the EFI data: >> >> .data : ALIGN(4096) { >> ... >> . = ALIGN(512); >> } >> >> which is aligned to 4K but aligns the end of itself to 512. >> >> So, we have the end of .got aligned to 4, followed by .pad that tries to >> align to 8, followed by an optional .data section. This is pointless. >> >> A sane patch would be to choose between the EFI .data section and the >> .pad section. So, it should be: >> >> #ifdef CONFIG_EFI_STUB >> .data : ALIGN(4096) { >> ... >> . = ALIGN(512); >> } >> #else >> .pad : { BYTE(0); . = ALIGN(8); } >> #endif >> > > Agreed, the .pad section has no point for EFI_STUB=y. However, it > seems this symptom is caused by the same issues I am trying to address > here > > https://marc.info/?l=linux-arm-kernel&m=150488477807353 > > which is that we have __ksymtab_xxx sections that we should discard, > because the linker will otherwise emit them /after/ .data or .pad. > This is caused by the use of lib/sort.c in the EFI stub, which > contains an EXPORT_SYMBOL(). hmm, right, didn't notice the data is already aligned... so it's indeed caused by the ksym: [ 9] .data PROGBITS 006ce000 6d6000 000200 00 WA 0 0 4096 [10] ___ksymtab+sort PROGBITS 006ce200 6d6200 000008 00 WA 0 0 4 [11] .bss NOBITS 006ce208 6d6208 00001c 00 WA 0 0 4 and both of your old([PATCH] ARM: compressed: discard ksym/kcrctab input section) and new([PATCH] efi/libstub: arm: omit sorting of the UEFI memory map) patches fix the issue i meet, thanks:) > > Would you perhaps prefer that I clone sort.c into its own .c file > specifically for the EFI stub? (under drivers/firmware/efi/libstub) > That should get rid of these spurious sections and thus the > misalignments and/or movements that are causing all of these issues. > > > ^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH] ARM: Fix zImage file size not aligned with CONFIG_EFI_STUB enabled 2017-10-23 3:26 ` jeffy @ 2017-10-23 8:50 ` Russell King - ARM Linux -1 siblings, 0 replies; 42+ messages in thread From: Russell King - ARM Linux @ 2017-10-23 8:50 UTC (permalink / raw) To: linux-arm-kernel On Mon, Oct 23, 2017 at 11:26:49AM +0800, jeffy wrote: > Hi Ard, > > On 10/22/2017 09:01 PM, Ard Biesheuvel wrote: > >On 22 October 2017 at 13:47, Russell King - ARM Linux > ><linux@armlinux.org.uk> wrote: > >>On Sun, Oct 22, 2017 at 12:01:13PM +0100, Ard Biesheuvel wrote: > >>>On 18 October 2017 at 06:01, Jeffy Chen <jeffy.chen@rock-chips.com> wrote: > >>>>The zImage file size should be aligned. > >>>> > >>>>Fixes: e4bae4d0b5f3 ("arm/efi: Split zImage code and data into separate PE/COFF sections") > >>>>Signed-off-by: Jeffy Chen <jeffy.chen@rock-chips.com> > >>>>--- > >>>> > >>>> arch/arm/boot/compressed/vmlinux.lds.S | 8 ++++---- > >>>> 1 file changed, 4 insertions(+), 4 deletions(-) > >>>> > >>>>diff --git a/arch/arm/boot/compressed/vmlinux.lds.S b/arch/arm/boot/compressed/vmlinux.lds.S > >>>>index b38dcef90756..1636fa259577 100644 > >>>>--- a/arch/arm/boot/compressed/vmlinux.lds.S > >>>>+++ b/arch/arm/boot/compressed/vmlinux.lds.S > >>>>@@ -70,10 +70,6 @@ SECTIONS > >>>> .got : { *(.got) } > >>>> _got_end = .; > >>>> > >>>>- /* ensure the zImage file size is always a multiple of 64 bits */ > >>>>- /* (without a dummy byte, ld just ignores the empty section) */ > >>>>- .pad : { BYTE(0); . = ALIGN(8); } > >>>>- > >>>> #ifdef CONFIG_EFI_STUB > >>>> .data : ALIGN(4096) { > >>>> __pecoff_data_start = .; > >>>>@@ -93,6 +89,10 @@ SECTIONS > >>>> __pecoff_data_rawsize = . - ADDR(.data); > >>>> #endif > >>>> > >>>>+ /* ensure the zImage file size is always a multiple of 64 bits */ > >>>>+ /* (without a dummy byte, ld just ignores the empty section) */ > >>>>+ .pad : { BYTE(0); . = ALIGN(8); } > >>>>+ > >>>> _edata = .; > >>>> > >>>> _magic_sig = ZIMAGE_MAGIC(0x016f2818); > >>>>-- > >>>>2.11.0 > >>>> > >>> > >>>This is not the right fix. If CONFIG_EFI_STUB is enabled, the zImage > >>>filesize should be rounded up to 512 bytes not 8 bytes. The '. = > >>>ALIGN(512);' in the .data section appears to ensure that, but for some > >>>reason, that appears not to be working. > >> > >>Actually, the existing .pad section is totally and utterly bogus when > >>EFI is enabled: > >> > >> . = ALIGN(4); > >> _etext = .; > >> > >> .got.plt : { *(.got.plt) } > >> _got_start = .; > >> .got : { *(.got) } > >> _got_end = .; > >> > >>The .got.plt and .got are always word-based. This is then followed by > >>.pad, which does nothing but pad out to a multiple of 64 bit: > >> > >> /* ensure the zImage file size is always a multiple of 64 bits */ > >> /* (without a dummy byte, ld just ignores the empty section) */ > >> .pad : { BYTE(0); . = ALIGN(8); } > >> > >>So this may add zero or 4 bytes of padding. > >> > >>This is then followed by the EFI data: > >> > >> .data : ALIGN(4096) { > >> ... > >> . = ALIGN(512); > >> } > >> > >>which is aligned to 4K but aligns the end of itself to 512. > >> > >>So, we have the end of .got aligned to 4, followed by .pad that tries to > >>align to 8, followed by an optional .data section. This is pointless. > >> > >>A sane patch would be to choose between the EFI .data section and the > >>.pad section. So, it should be: > >> > >>#ifdef CONFIG_EFI_STUB > >> .data : ALIGN(4096) { > >> ... > >> . = ALIGN(512); > >> } > >>#else > >> .pad : { BYTE(0); . = ALIGN(8); } > >>#endif > >> > > > >Agreed, the .pad section has no point for EFI_STUB=y. However, it > >seems this symptom is caused by the same issues I am trying to address > >here > > > >https://marc.info/?l=linux-arm-kernel&m=150488477807353 > > > >which is that we have __ksymtab_xxx sections that we should discard, > >because the linker will otherwise emit them /after/ .data or .pad. > >This is caused by the use of lib/sort.c in the EFI stub, which > >contains an EXPORT_SYMBOL(). > > hmm, right, didn't notice the data is already aligned... > so it's indeed caused by the ksym: > > [ 9] .data PROGBITS 006ce000 6d6000 000200 00 WA 0 > 0 4096 > [10] ___ksymtab+sort PROGBITS 006ce200 6d6200 000008 00 WA 0 > 0 4 > [11] .bss NOBITS 006ce208 6d6208 00001c 00 WA 0 > 0 4 It's earlier - look for __ksymtab_strings. -- RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up According to speedtest.net: 8.21Mbps down 510kbps up ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH] ARM: Fix zImage file size not aligned with CONFIG_EFI_STUB enabled @ 2017-10-23 8:50 ` Russell King - ARM Linux 0 siblings, 0 replies; 42+ messages in thread From: Russell King - ARM Linux @ 2017-10-23 8:50 UTC (permalink / raw) To: jeffy Cc: Ard Biesheuvel, linux-kernel@vger.kernel.org, chris.zhong, Ingo Molnar, linux-arm-kernel@lists.infradead.org On Mon, Oct 23, 2017 at 11:26:49AM +0800, jeffy wrote: > Hi Ard, > > On 10/22/2017 09:01 PM, Ard Biesheuvel wrote: > >On 22 October 2017 at 13:47, Russell King - ARM Linux > ><linux@armlinux.org.uk> wrote: > >>On Sun, Oct 22, 2017 at 12:01:13PM +0100, Ard Biesheuvel wrote: > >>>On 18 October 2017 at 06:01, Jeffy Chen <jeffy.chen@rock-chips.com> wrote: > >>>>The zImage file size should be aligned. > >>>> > >>>>Fixes: e4bae4d0b5f3 ("arm/efi: Split zImage code and data into separate PE/COFF sections") > >>>>Signed-off-by: Jeffy Chen <jeffy.chen@rock-chips.com> > >>>>--- > >>>> > >>>> arch/arm/boot/compressed/vmlinux.lds.S | 8 ++++---- > >>>> 1 file changed, 4 insertions(+), 4 deletions(-) > >>>> > >>>>diff --git a/arch/arm/boot/compressed/vmlinux.lds.S b/arch/arm/boot/compressed/vmlinux.lds.S > >>>>index b38dcef90756..1636fa259577 100644 > >>>>--- a/arch/arm/boot/compressed/vmlinux.lds.S > >>>>+++ b/arch/arm/boot/compressed/vmlinux.lds.S > >>>>@@ -70,10 +70,6 @@ SECTIONS > >>>> .got : { *(.got) } > >>>> _got_end = .; > >>>> > >>>>- /* ensure the zImage file size is always a multiple of 64 bits */ > >>>>- /* (without a dummy byte, ld just ignores the empty section) */ > >>>>- .pad : { BYTE(0); . = ALIGN(8); } > >>>>- > >>>> #ifdef CONFIG_EFI_STUB > >>>> .data : ALIGN(4096) { > >>>> __pecoff_data_start = .; > >>>>@@ -93,6 +89,10 @@ SECTIONS > >>>> __pecoff_data_rawsize = . - ADDR(.data); > >>>> #endif > >>>> > >>>>+ /* ensure the zImage file size is always a multiple of 64 bits */ > >>>>+ /* (without a dummy byte, ld just ignores the empty section) */ > >>>>+ .pad : { BYTE(0); . = ALIGN(8); } > >>>>+ > >>>> _edata = .; > >>>> > >>>> _magic_sig = ZIMAGE_MAGIC(0x016f2818); > >>>>-- > >>>>2.11.0 > >>>> > >>> > >>>This is not the right fix. If CONFIG_EFI_STUB is enabled, the zImage > >>>filesize should be rounded up to 512 bytes not 8 bytes. The '. = > >>>ALIGN(512);' in the .data section appears to ensure that, but for some > >>>reason, that appears not to be working. > >> > >>Actually, the existing .pad section is totally and utterly bogus when > >>EFI is enabled: > >> > >> . = ALIGN(4); > >> _etext = .; > >> > >> .got.plt : { *(.got.plt) } > >> _got_start = .; > >> .got : { *(.got) } > >> _got_end = .; > >> > >>The .got.plt and .got are always word-based. This is then followed by > >>.pad, which does nothing but pad out to a multiple of 64 bit: > >> > >> /* ensure the zImage file size is always a multiple of 64 bits */ > >> /* (without a dummy byte, ld just ignores the empty section) */ > >> .pad : { BYTE(0); . = ALIGN(8); } > >> > >>So this may add zero or 4 bytes of padding. > >> > >>This is then followed by the EFI data: > >> > >> .data : ALIGN(4096) { > >> ... > >> . = ALIGN(512); > >> } > >> > >>which is aligned to 4K but aligns the end of itself to 512. > >> > >>So, we have the end of .got aligned to 4, followed by .pad that tries to > >>align to 8, followed by an optional .data section. This is pointless. > >> > >>A sane patch would be to choose between the EFI .data section and the > >>.pad section. So, it should be: > >> > >>#ifdef CONFIG_EFI_STUB > >> .data : ALIGN(4096) { > >> ... > >> . = ALIGN(512); > >> } > >>#else > >> .pad : { BYTE(0); . = ALIGN(8); } > >>#endif > >> > > > >Agreed, the .pad section has no point for EFI_STUB=y. However, it > >seems this symptom is caused by the same issues I am trying to address > >here > > > >https://marc.info/?l=linux-arm-kernel&m=150488477807353 > > > >which is that we have __ksymtab_xxx sections that we should discard, > >because the linker will otherwise emit them /after/ .data or .pad. > >This is caused by the use of lib/sort.c in the EFI stub, which > >contains an EXPORT_SYMBOL(). > > hmm, right, didn't notice the data is already aligned... > so it's indeed caused by the ksym: > > [ 9] .data PROGBITS 006ce000 6d6000 000200 00 WA 0 > 0 4096 > [10] ___ksymtab+sort PROGBITS 006ce200 6d6200 000008 00 WA 0 > 0 4 > [11] .bss NOBITS 006ce208 6d6208 00001c 00 WA 0 > 0 4 It's earlier - look for __ksymtab_strings. -- RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up According to speedtest.net: 8.21Mbps down 510kbps up ^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH] ARM: Fix zImage file size not aligned with CONFIG_EFI_STUB enabled 2017-10-23 8:50 ` Russell King - ARM Linux @ 2017-10-23 10:24 ` jeffy -1 siblings, 0 replies; 42+ messages in thread From: jeffy @ 2017-10-23 10:24 UTC (permalink / raw) To: linux-arm-kernel Hi Russell, Thanks for your reply. On 10/23/2017 04:50 PM, Russell King - ARM Linux wrote: >> > >> >hmm, right, didn't notice the data is already aligned... >> >so it's indeed caused by the ksym: >> > >> > [ 9] .data PROGBITS 006ce000 6d6000 000200 00 WA 0 >> >0 4096 >> > [10] ___ksymtab+sort PROGBITS 006ce200 6d6200 000008 00 WA 0 >> >0 4 >> > [11] .bss NOBITS 006ce208 6d6208 00001c 00 WA 0 >> >0 4 > It's earlier - look for __ksymtab_strings. the problem i meet is the appended dtb code found dtb invalid. i thought that is because of unaligned zImage size, but i was wrong... it looks like the size is still aligned, and after add more logs, it seems the problem is due to _edata not matched the real file size, which is because of the unexpected ___ksymtab+sort: currently: zImage size is 6d6208: -rwxr-xr-x 1 root root 7135752 Oct 23 18:12 zImage _edata is 006ce200: 006ce200 0 NOTYPE GLOBAL DEFAULT 9 _edata vmlinux sections: Section Headers: [Nr] Name Type Addr Off Size ES Flg Lk Inf Al [ 0] NULL 00000000 000000 000000 00 0 0 0 [ 1] .text PROGBITS 00000000 008000 00b7a0 00 AX 0 0 4096 [ 2] .table PROGBITS 0000b7a0 0137a0 000014 00 WA 0 0 4 [ 3] .rodata PROGBITS 0000b7b4 0137b4 0015ef 00 A 0 0 2 [ 4] __ksymtab_strings PROGBITS 0000cda3 014da3 000005 00 A 0 0 1 [ 5] .piggydata PROGBITS 0000cda8 014da8 6c026f 00 A 0 0 1 [ 6] .got.plt PROGBITS 006cd018 6d5018 00000c 04 WA 0 0 4 [ 7] .got PROGBITS 006cd024 6d5024 000028 00 WA 0 0 4 [ 8] .pad PROGBITS 006cd04c 6d504c 000004 00 WA 0 0 1 [ 9] .data PROGBITS 006ce000 6d6000 000200 00 WA 0 0 4096 [10] ___ksymtab+sort PROGBITS 006ce200 6d6200 000008 00 WA 0 0 4 [11] .bss NOBITS 006ce208 6d6208 00001c 00 WA 0 0 4 and it turns out moving around .pad section only hide the problem by placing the .pad after the ___ksymtab+sort: Section Headers: [Nr] Name Type Addr Off Size ES Flg Lk Inf Al [ 0] NULL 00000000 000000 000000 00 0 0 0 [ 1] .text PROGBITS 00000000 008000 00b7a0 00 AX 0 0 4096 [ 2] .table PROGBITS 0000b7a0 0137a0 000014 00 WA 0 0 4 [ 3] .rodata PROGBITS 0000b7b4 0137b4 0015ef 00 A 0 0 2 [ 4] __ksymtab_strings PROGBITS 0000cda3 014da3 000005 00 A 0 0 1 [ 5] .piggydata PROGBITS 0000cda8 014da8 6c026f 00 A 0 0 1 [ 6] .got.plt PROGBITS 006cd018 6d5018 00000c 04 WA 0 0 4 [ 7] .got PROGBITS 006cd024 6d5024 000028 00 WA 0 0 4 [ 8] .data PROGBITS 006ce000 6d6000 000200 00 WA 0 0 4096 [ 9] ___ksymtab+sort PROGBITS 006ce200 6d6200 000008 00 WA 0 0 4 [10] .pad PROGBITS 006ce208 6d6208 000008 00 WA 0 0 1 [11] .bss NOBITS 006ce210 6d6210 00001c 00 WA 0 0 4 -rwxr-xr-x 1 root root 7135760 Oct 23 18:09 zImage 006ce210 0 NOTYPE GLOBAL DEFAULT 10 _edata and i think Ard's new patch could be the right way to fix it :) ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH] ARM: Fix zImage file size not aligned with CONFIG_EFI_STUB enabled @ 2017-10-23 10:24 ` jeffy 0 siblings, 0 replies; 42+ messages in thread From: jeffy @ 2017-10-23 10:24 UTC (permalink / raw) To: Russell King - ARM Linux Cc: Ard Biesheuvel, linux-kernel@vger.kernel.org, chris.zhong, Ingo Molnar, linux-arm-kernel@lists.infradead.org Hi Russell, Thanks for your reply. On 10/23/2017 04:50 PM, Russell King - ARM Linux wrote: >> > >> >hmm, right, didn't notice the data is already aligned... >> >so it's indeed caused by the ksym: >> > >> > [ 9] .data PROGBITS 006ce000 6d6000 000200 00 WA 0 >> >0 4096 >> > [10] ___ksymtab+sort PROGBITS 006ce200 6d6200 000008 00 WA 0 >> >0 4 >> > [11] .bss NOBITS 006ce208 6d6208 00001c 00 WA 0 >> >0 4 > It's earlier - look for __ksymtab_strings. the problem i meet is the appended dtb code found dtb invalid. i thought that is because of unaligned zImage size, but i was wrong... it looks like the size is still aligned, and after add more logs, it seems the problem is due to _edata not matched the real file size, which is because of the unexpected ___ksymtab+sort: currently: zImage size is 6d6208: -rwxr-xr-x 1 root root 7135752 Oct 23 18:12 zImage _edata is 006ce200: 006ce200 0 NOTYPE GLOBAL DEFAULT 9 _edata vmlinux sections: Section Headers: [Nr] Name Type Addr Off Size ES Flg Lk Inf Al [ 0] NULL 00000000 000000 000000 00 0 0 0 [ 1] .text PROGBITS 00000000 008000 00b7a0 00 AX 0 0 4096 [ 2] .table PROGBITS 0000b7a0 0137a0 000014 00 WA 0 0 4 [ 3] .rodata PROGBITS 0000b7b4 0137b4 0015ef 00 A 0 0 2 [ 4] __ksymtab_strings PROGBITS 0000cda3 014da3 000005 00 A 0 0 1 [ 5] .piggydata PROGBITS 0000cda8 014da8 6c026f 00 A 0 0 1 [ 6] .got.plt PROGBITS 006cd018 6d5018 00000c 04 WA 0 0 4 [ 7] .got PROGBITS 006cd024 6d5024 000028 00 WA 0 0 4 [ 8] .pad PROGBITS 006cd04c 6d504c 000004 00 WA 0 0 1 [ 9] .data PROGBITS 006ce000 6d6000 000200 00 WA 0 0 4096 [10] ___ksymtab+sort PROGBITS 006ce200 6d6200 000008 00 WA 0 0 4 [11] .bss NOBITS 006ce208 6d6208 00001c 00 WA 0 0 4 and it turns out moving around .pad section only hide the problem by placing the .pad after the ___ksymtab+sort: Section Headers: [Nr] Name Type Addr Off Size ES Flg Lk Inf Al [ 0] NULL 00000000 000000 000000 00 0 0 0 [ 1] .text PROGBITS 00000000 008000 00b7a0 00 AX 0 0 4096 [ 2] .table PROGBITS 0000b7a0 0137a0 000014 00 WA 0 0 4 [ 3] .rodata PROGBITS 0000b7b4 0137b4 0015ef 00 A 0 0 2 [ 4] __ksymtab_strings PROGBITS 0000cda3 014da3 000005 00 A 0 0 1 [ 5] .piggydata PROGBITS 0000cda8 014da8 6c026f 00 A 0 0 1 [ 6] .got.plt PROGBITS 006cd018 6d5018 00000c 04 WA 0 0 4 [ 7] .got PROGBITS 006cd024 6d5024 000028 00 WA 0 0 4 [ 8] .data PROGBITS 006ce000 6d6000 000200 00 WA 0 0 4096 [ 9] ___ksymtab+sort PROGBITS 006ce200 6d6200 000008 00 WA 0 0 4 [10] .pad PROGBITS 006ce208 6d6208 000008 00 WA 0 0 1 [11] .bss NOBITS 006ce210 6d6210 00001c 00 WA 0 0 4 -rwxr-xr-x 1 root root 7135760 Oct 23 18:09 zImage 006ce210 0 NOTYPE GLOBAL DEFAULT 10 _edata and i think Ard's new patch could be the right way to fix it :) ^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH] ARM: Fix zImage file size not aligned with CONFIG_EFI_STUB enabled 2017-10-23 10:24 ` jeffy @ 2017-10-23 10:50 ` Russell King - ARM Linux -1 siblings, 0 replies; 42+ messages in thread From: Russell King - ARM Linux @ 2017-10-23 10:50 UTC (permalink / raw) To: linux-arm-kernel On Mon, Oct 23, 2017 at 06:24:12PM +0800, jeffy wrote: > Hi Russell, > > Thanks for your reply. > > On 10/23/2017 04:50 PM, Russell King - ARM Linux wrote: > >>> > >>>hmm, right, didn't notice the data is already aligned... > >>>so it's indeed caused by the ksym: > >>> > >>> [ 9] .data PROGBITS 006ce000 6d6000 000200 00 WA 0 > >>>0 4096 > >>> [10] ___ksymtab+sort PROGBITS 006ce200 6d6200 000008 00 WA 0 > >>>0 4 > >>> [11] .bss NOBITS 006ce208 6d6208 00001c 00 WA 0 > >>>0 4 > >It's earlier - look for __ksymtab_strings. > > the problem i meet is the appended dtb code found dtb invalid. i thought > that is because of unaligned zImage size, but i was wrong... Hmm, you really ought not to be using the appended dtb code for modern systems - the appended dtb system is there for old boot loaders that are incapable of dealing with a dtb. As is said in the option's help text: This is meant as a backward compatibility convenience for those systems with a bootloader that can't be upgraded to accommodate the documented boot protocol using a device tree. Beware that there is very little in terms of protection against this option being confused by leftover garbage in memory that might look like a DTB header after a reboot if no actual DTB is appended to zImage. Do not leave this option active in a production kernel if you don't intend to always append a DTB. Proper passing of the location into r2 of a bootloader provided DTB is always preferable to this option. If you rely on it, and you have something that looks like a dtb after the image, then things will go wrong, so it's better _not_ to use it and to keep it disabled. That aside, thanks for doing a more in-depth analysis of what is going on, which helps to understand /why/ Ard's fix works (whereas before it was rather nebulous.) I wonder whether we ought to tell the linker to discard any unknown sections by adding at the bottom: /DISCARD/ { *(*) } but I do think we need to document this, specifically that _edata must point to the first byte after the binary file, and that the only sections after it are allowed to be the .bss and stack sections. -- RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up According to speedtest.net: 8.21Mbps down 510kbps up ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH] ARM: Fix zImage file size not aligned with CONFIG_EFI_STUB enabled @ 2017-10-23 10:50 ` Russell King - ARM Linux 0 siblings, 0 replies; 42+ messages in thread From: Russell King - ARM Linux @ 2017-10-23 10:50 UTC (permalink / raw) To: jeffy Cc: Ard Biesheuvel, linux-kernel@vger.kernel.org, chris.zhong, Ingo Molnar, linux-arm-kernel@lists.infradead.org On Mon, Oct 23, 2017 at 06:24:12PM +0800, jeffy wrote: > Hi Russell, > > Thanks for your reply. > > On 10/23/2017 04:50 PM, Russell King - ARM Linux wrote: > >>> > >>>hmm, right, didn't notice the data is already aligned... > >>>so it's indeed caused by the ksym: > >>> > >>> [ 9] .data PROGBITS 006ce000 6d6000 000200 00 WA 0 > >>>0 4096 > >>> [10] ___ksymtab+sort PROGBITS 006ce200 6d6200 000008 00 WA 0 > >>>0 4 > >>> [11] .bss NOBITS 006ce208 6d6208 00001c 00 WA 0 > >>>0 4 > >It's earlier - look for __ksymtab_strings. > > the problem i meet is the appended dtb code found dtb invalid. i thought > that is because of unaligned zImage size, but i was wrong... Hmm, you really ought not to be using the appended dtb code for modern systems - the appended dtb system is there for old boot loaders that are incapable of dealing with a dtb. As is said in the option's help text: This is meant as a backward compatibility convenience for those systems with a bootloader that can't be upgraded to accommodate the documented boot protocol using a device tree. Beware that there is very little in terms of protection against this option being confused by leftover garbage in memory that might look like a DTB header after a reboot if no actual DTB is appended to zImage. Do not leave this option active in a production kernel if you don't intend to always append a DTB. Proper passing of the location into r2 of a bootloader provided DTB is always preferable to this option. If you rely on it, and you have something that looks like a dtb after the image, then things will go wrong, so it's better _not_ to use it and to keep it disabled. That aside, thanks for doing a more in-depth analysis of what is going on, which helps to understand /why/ Ard's fix works (whereas before it was rather nebulous.) I wonder whether we ought to tell the linker to discard any unknown sections by adding at the bottom: /DISCARD/ { *(*) } but I do think we need to document this, specifically that _edata must point to the first byte after the binary file, and that the only sections after it are allowed to be the .bss and stack sections. -- RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up According to speedtest.net: 8.21Mbps down 510kbps up ^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH] ARM: Fix zImage file size not aligned with CONFIG_EFI_STUB enabled 2017-10-23 10:50 ` Russell King - ARM Linux @ 2017-10-23 11:45 ` Russell King - ARM Linux -1 siblings, 0 replies; 42+ messages in thread From: Russell King - ARM Linux @ 2017-10-23 11:45 UTC (permalink / raw) To: linux-arm-kernel On Mon, Oct 23, 2017 at 11:50:46AM +0100, Russell King - ARM Linux wrote: > On Mon, Oct 23, 2017 at 06:24:12PM +0800, jeffy wrote: > > Hi Russell, > > > > Thanks for your reply. > > > > On 10/23/2017 04:50 PM, Russell King - ARM Linux wrote: > > >>> > > >>>hmm, right, didn't notice the data is already aligned... > > >>>so it's indeed caused by the ksym: > > >>> > > >>> [ 9] .data PROGBITS 006ce000 6d6000 000200 00 WA 0 > > >>>0 4096 > > >>> [10] ___ksymtab+sort PROGBITS 006ce200 6d6200 000008 00 WA 0 > > >>>0 4 > > >>> [11] .bss NOBITS 006ce208 6d6208 00001c 00 WA 0 > > >>>0 4 > > >It's earlier - look for __ksymtab_strings. > > > > the problem i meet is the appended dtb code found dtb invalid. i thought > > that is because of unaligned zImage size, but i was wrong... > > Hmm, you really ought not to be using the appended dtb code for modern > systems - the appended dtb system is there for old boot loaders that > are incapable of dealing with a dtb. As is said in the option's help > text: > > This is meant as a backward compatibility convenience for those > systems with a bootloader that can't be upgraded to accommodate > the documented boot protocol using a device tree. > > Beware that there is very little in terms of protection against > this option being confused by leftover garbage in memory that might > look like a DTB header after a reboot if no actual DTB is appended > to zImage. Do not leave this option active in a production kernel > if you don't intend to always append a DTB. Proper passing of the > location into r2 of a bootloader provided DTB is always preferable > to this option. > > If you rely on it, and you have something that looks like a dtb after > the image, then things will go wrong, so it's better _not_ to use it > and to keep it disabled. > > That aside, thanks for doing a more in-depth analysis of what is going > on, which helps to understand /why/ Ard's fix works (whereas before > it was rather nebulous.) > > I wonder whether we ought to tell the linker to discard any unknown > sections by adding at the bottom: > > /DISCARD/ { *(*) } > > but I do think we need to document this, specifically that _edata must > point to the first byte after the binary file, and that the only > sections after it are allowed to be the .bss and stack sections. Short of adding the discard (which I think is itself risky, we've had problems in the main kernel's vmlinux.lds in this area), I think we ought to verify the size of the zImage file, so that the build fails when we generate a zImage which is wrong, rather than producing a zImage that is incorrect. Maybe something like this? 8<===== From: Russell King <rmk+kernel@armlinux.org.uk> Subject: [PATCH] ARM: verify size of zImage The linker can sometimes add additional sections to the zImage ELF file which results in the zImage binary being larger than expected. This causes appended DT blobs to fail. Verify that the zImage binary is the expected size, and fail the build if this is not the case. Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk> --- arch/arm/boot/Makefile | 6 +++++- arch/arm/boot/verify_zimage.sh | 21 +++++++++++++++++++++ 2 files changed, 26 insertions(+), 1 deletion(-) create mode 100755 arch/arm/boot/verify_zimage.sh diff --git a/arch/arm/boot/Makefile b/arch/arm/boot/Makefile index a3af4dc08c3e..1290875556ae 100644 --- a/arch/arm/boot/Makefile +++ b/arch/arm/boot/Makefile @@ -53,6 +53,10 @@ $(obj)/Image $(obj)/zImage: FORCE else +quiet_cmd_mkzimage = ZIMAGE $@ +cmd_mkzimage = $(cmd_objcopy) && $(CONFIG_SHELL) -c \ + '$(srctree)/$(src)/verify_zimage.sh $< $@ "$(NM)" || { rm -f $@; false; }' + $(obj)/xipImage: FORCE @echo 'Kernel not configured for XIP (CONFIG_XIP_KERNEL!=y)' @false @@ -64,7 +68,7 @@ $(obj)/compressed/vmlinux: $(obj)/Image FORCE $(Q)$(MAKE) $(build)=$(obj)/compressed $@ $(obj)/zImage: $(obj)/compressed/vmlinux FORCE - $(call if_changed,objcopy) + $(call if_changed,mkzimage) endif diff --git a/arch/arm/boot/verify_zimage.sh b/arch/arm/boot/verify_zimage.sh new file mode 100755 index 000000000000..922a93e61aa7 --- /dev/null +++ b/arch/arm/boot/verify_zimage.sh @@ -0,0 +1,21 @@ +#!/bin/sh +set -e + +vmlinuz="$1" +zimage="$2" +nm="$3" + +magic_size=$("$nm" "$vmlinuz" | perl -e 'while (<>) { + $magic_start = hex($1) if /^([[:xdigit:]]+) . _magic_start$/; + $magic_end = hex($1) if /^([[:xdigit:]]+) . _magic_end$/; +}; printf "%d\n", $magic_end - $magic_start;') + +zimage_size=$(stat -c '%s' "$zimage") + +# Verify that the resulting binary matches the size contained within +# the binary (iow, the linker has not added any additional sections.) +if [ $magic_size -ne $zimage_size ]; then + echo "zImage size ($zimage_size) disagrees with linked size ($magic_size)" >&2 + exit 1 +fi +exit 0 -- 2.7.4 -- RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up According to speedtest.net: 8.21Mbps down 510kbps up ^ permalink raw reply related [flat|nested] 42+ messages in thread
* Re: [PATCH] ARM: Fix zImage file size not aligned with CONFIG_EFI_STUB enabled @ 2017-10-23 11:45 ` Russell King - ARM Linux 0 siblings, 0 replies; 42+ messages in thread From: Russell King - ARM Linux @ 2017-10-23 11:45 UTC (permalink / raw) To: jeffy Cc: Ingo Molnar, chris.zhong, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, Ard Biesheuvel On Mon, Oct 23, 2017 at 11:50:46AM +0100, Russell King - ARM Linux wrote: > On Mon, Oct 23, 2017 at 06:24:12PM +0800, jeffy wrote: > > Hi Russell, > > > > Thanks for your reply. > > > > On 10/23/2017 04:50 PM, Russell King - ARM Linux wrote: > > >>> > > >>>hmm, right, didn't notice the data is already aligned... > > >>>so it's indeed caused by the ksym: > > >>> > > >>> [ 9] .data PROGBITS 006ce000 6d6000 000200 00 WA 0 > > >>>0 4096 > > >>> [10] ___ksymtab+sort PROGBITS 006ce200 6d6200 000008 00 WA 0 > > >>>0 4 > > >>> [11] .bss NOBITS 006ce208 6d6208 00001c 00 WA 0 > > >>>0 4 > > >It's earlier - look for __ksymtab_strings. > > > > the problem i meet is the appended dtb code found dtb invalid. i thought > > that is because of unaligned zImage size, but i was wrong... > > Hmm, you really ought not to be using the appended dtb code for modern > systems - the appended dtb system is there for old boot loaders that > are incapable of dealing with a dtb. As is said in the option's help > text: > > This is meant as a backward compatibility convenience for those > systems with a bootloader that can't be upgraded to accommodate > the documented boot protocol using a device tree. > > Beware that there is very little in terms of protection against > this option being confused by leftover garbage in memory that might > look like a DTB header after a reboot if no actual DTB is appended > to zImage. Do not leave this option active in a production kernel > if you don't intend to always append a DTB. Proper passing of the > location into r2 of a bootloader provided DTB is always preferable > to this option. > > If you rely on it, and you have something that looks like a dtb after > the image, then things will go wrong, so it's better _not_ to use it > and to keep it disabled. > > That aside, thanks for doing a more in-depth analysis of what is going > on, which helps to understand /why/ Ard's fix works (whereas before > it was rather nebulous.) > > I wonder whether we ought to tell the linker to discard any unknown > sections by adding at the bottom: > > /DISCARD/ { *(*) } > > but I do think we need to document this, specifically that _edata must > point to the first byte after the binary file, and that the only > sections after it are allowed to be the .bss and stack sections. Short of adding the discard (which I think is itself risky, we've had problems in the main kernel's vmlinux.lds in this area), I think we ought to verify the size of the zImage file, so that the build fails when we generate a zImage which is wrong, rather than producing a zImage that is incorrect. Maybe something like this? 8<===== From: Russell King <rmk+kernel@armlinux.org.uk> Subject: [PATCH] ARM: verify size of zImage The linker can sometimes add additional sections to the zImage ELF file which results in the zImage binary being larger than expected. This causes appended DT blobs to fail. Verify that the zImage binary is the expected size, and fail the build if this is not the case. Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk> --- arch/arm/boot/Makefile | 6 +++++- arch/arm/boot/verify_zimage.sh | 21 +++++++++++++++++++++ 2 files changed, 26 insertions(+), 1 deletion(-) create mode 100755 arch/arm/boot/verify_zimage.sh diff --git a/arch/arm/boot/Makefile b/arch/arm/boot/Makefile index a3af4dc08c3e..1290875556ae 100644 --- a/arch/arm/boot/Makefile +++ b/arch/arm/boot/Makefile @@ -53,6 +53,10 @@ $(obj)/Image $(obj)/zImage: FORCE else +quiet_cmd_mkzimage = ZIMAGE $@ +cmd_mkzimage = $(cmd_objcopy) && $(CONFIG_SHELL) -c \ + '$(srctree)/$(src)/verify_zimage.sh $< $@ "$(NM)" || { rm -f $@; false; }' + $(obj)/xipImage: FORCE @echo 'Kernel not configured for XIP (CONFIG_XIP_KERNEL!=y)' @false @@ -64,7 +68,7 @@ $(obj)/compressed/vmlinux: $(obj)/Image FORCE $(Q)$(MAKE) $(build)=$(obj)/compressed $@ $(obj)/zImage: $(obj)/compressed/vmlinux FORCE - $(call if_changed,objcopy) + $(call if_changed,mkzimage) endif diff --git a/arch/arm/boot/verify_zimage.sh b/arch/arm/boot/verify_zimage.sh new file mode 100755 index 000000000000..922a93e61aa7 --- /dev/null +++ b/arch/arm/boot/verify_zimage.sh @@ -0,0 +1,21 @@ +#!/bin/sh +set -e + +vmlinuz="$1" +zimage="$2" +nm="$3" + +magic_size=$("$nm" "$vmlinuz" | perl -e 'while (<>) { + $magic_start = hex($1) if /^([[:xdigit:]]+) . _magic_start$/; + $magic_end = hex($1) if /^([[:xdigit:]]+) . _magic_end$/; +}; printf "%d\n", $magic_end - $magic_start;') + +zimage_size=$(stat -c '%s' "$zimage") + +# Verify that the resulting binary matches the size contained within +# the binary (iow, the linker has not added any additional sections.) +if [ $magic_size -ne $zimage_size ]; then + echo "zImage size ($zimage_size) disagrees with linked size ($magic_size)" >&2 + exit 1 +fi +exit 0 -- 2.7.4 -- RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up According to speedtest.net: 8.21Mbps down 510kbps up ^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH] ARM: Fix zImage file size not aligned with CONFIG_EFI_STUB enabled 2017-10-23 11:45 ` Russell King - ARM Linux @ 2017-10-24 8:09 ` Ard Biesheuvel -1 siblings, 0 replies; 42+ messages in thread From: Ard Biesheuvel @ 2017-10-24 8:09 UTC (permalink / raw) To: linux-arm-kernel On 23 October 2017 at 12:45, Russell King - ARM Linux <linux@armlinux.org.uk> wrote: > On Mon, Oct 23, 2017 at 11:50:46AM +0100, Russell King - ARM Linux wrote: >> On Mon, Oct 23, 2017 at 06:24:12PM +0800, jeffy wrote: >> > Hi Russell, >> > >> > Thanks for your reply. >> > >> > On 10/23/2017 04:50 PM, Russell King - ARM Linux wrote: >> > >>> >> > >>>hmm, right, didn't notice the data is already aligned... >> > >>>so it's indeed caused by the ksym: >> > >>> >> > >>> [ 9] .data PROGBITS 006ce000 6d6000 000200 00 WA 0 >> > >>>0 4096 >> > >>> [10] ___ksymtab+sort PROGBITS 006ce200 6d6200 000008 00 WA 0 >> > >>>0 4 >> > >>> [11] .bss NOBITS 006ce208 6d6208 00001c 00 WA 0 >> > >>>0 4 >> > >It's earlier - look for __ksymtab_strings. >> > >> > the problem i meet is the appended dtb code found dtb invalid. i thought >> > that is because of unaligned zImage size, but i was wrong... >> >> Hmm, you really ought not to be using the appended dtb code for modern >> systems - the appended dtb system is there for old boot loaders that >> are incapable of dealing with a dtb. As is said in the option's help >> text: >> >> This is meant as a backward compatibility convenience for those >> systems with a bootloader that can't be upgraded to accommodate >> the documented boot protocol using a device tree. >> >> Beware that there is very little in terms of protection against >> this option being confused by leftover garbage in memory that might >> look like a DTB header after a reboot if no actual DTB is appended >> to zImage. Do not leave this option active in a production kernel >> if you don't intend to always append a DTB. Proper passing of the >> location into r2 of a bootloader provided DTB is always preferable >> to this option. >> >> If you rely on it, and you have something that looks like a dtb after >> the image, then things will go wrong, so it's better _not_ to use it >> and to keep it disabled. >> >> That aside, thanks for doing a more in-depth analysis of what is going >> on, which helps to understand /why/ Ard's fix works (whereas before >> it was rather nebulous.) >> >> I wonder whether we ought to tell the linker to discard any unknown >> sections by adding at the bottom: >> >> /DISCARD/ { *(*) } >> >> but I do think we need to document this, specifically that _edata must >> point to the first byte after the binary file, and that the only >> sections after it are allowed to be the .bss and stack sections. > > Short of adding the discard (which I think is itself risky, we've had > problems in the main kernel's vmlinux.lds in this area), I think we > ought to verify the size of the zImage file, so that the build fails > when we generate a zImage which is wrong, rather than producing a > zImage that is incorrect. Maybe something like this? > > 8<===== > From: Russell King <rmk+kernel@armlinux.org.uk> > Subject: [PATCH] ARM: verify size of zImage > > The linker can sometimes add additional sections to the zImage ELF file > which results in the zImage binary being larger than expected. This > causes appended DT blobs to fail. > > Verify that the zImage binary is the expected size, and fail the build > if this is not the case. > > Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk> > --- > arch/arm/boot/Makefile | 6 +++++- > arch/arm/boot/verify_zimage.sh | 21 +++++++++++++++++++++ > 2 files changed, 26 insertions(+), 1 deletion(-) > create mode 100755 arch/arm/boot/verify_zimage.sh > > diff --git a/arch/arm/boot/Makefile b/arch/arm/boot/Makefile > index a3af4dc08c3e..1290875556ae 100644 > --- a/arch/arm/boot/Makefile > +++ b/arch/arm/boot/Makefile > @@ -53,6 +53,10 @@ $(obj)/Image $(obj)/zImage: FORCE > > else > > +quiet_cmd_mkzimage = ZIMAGE $@ > +cmd_mkzimage = $(cmd_objcopy) && $(CONFIG_SHELL) -c \ > + '$(srctree)/$(src)/verify_zimage.sh $< $@ "$(NM)" || { rm -f $@; false; }' > + > $(obj)/xipImage: FORCE > @echo 'Kernel not configured for XIP (CONFIG_XIP_KERNEL!=y)' > @false > @@ -64,7 +68,7 @@ $(obj)/compressed/vmlinux: $(obj)/Image FORCE > $(Q)$(MAKE) $(build)=$(obj)/compressed $@ > > $(obj)/zImage: $(obj)/compressed/vmlinux FORCE > - $(call if_changed,objcopy) > + $(call if_changed,mkzimage) > > endif > > diff --git a/arch/arm/boot/verify_zimage.sh b/arch/arm/boot/verify_zimage.sh > new file mode 100755 > index 000000000000..922a93e61aa7 > --- /dev/null > +++ b/arch/arm/boot/verify_zimage.sh > @@ -0,0 +1,21 @@ > +#!/bin/sh > +set -e > + > +vmlinuz="$1" > +zimage="$2" > +nm="$3" > + > +magic_size=$("$nm" "$vmlinuz" | perl -e 'while (<>) { > + $magic_start = hex($1) if /^([[:xdigit:]]+) . _magic_start$/; > + $magic_end = hex($1) if /^([[:xdigit:]]+) . _magic_end$/; > +}; printf "%d\n", $magic_end - $magic_start;') > + > +zimage_size=$(stat -c '%s' "$zimage") > + > +# Verify that the resulting binary matches the size contained within > +# the binary (iow, the linker has not added any additional sections.) > +if [ $magic_size -ne $zimage_size ]; then > + echo "zImage size ($zimage_size) disagrees with linked size ($magic_size)" >&2 > + exit 1 > +fi > +exit 0 > -- > 2.7.4 > The following patch appears to fix the issue as well: diff --git a/arch/arm/boot/compressed/vmlinux.lds.S b/arch/arm/boot/compressed/vmlinux.lds.S index 7a4c59154361..0e0f504e256e 100644 --- a/arch/arm/boot/compressed/vmlinux.lds.S +++ b/arch/arm/boot/compressed/vmlinux.lds.S @@ -83,7 +83,9 @@ SECTIONS __pecoff_data_rawsize = . - ADDR(.data); #endif - _edata = .; + .edata (NOLOAD) : { + _edata = .; + } _magic_sig = ZIMAGE_MAGIC(0x016f2818); _magic_start = ZIMAGE_MAGIC(_start); because the NOLOAD triggers the linker to emit all PROGBITS sections before _edata, including unknown ones. E.g., in my binary, it gives me 00798020 D __pecoff_data_end 00798200 d __ksymtab_sort 00798208 B __bss_start 00798208 D _edata ^ permalink raw reply related [flat|nested] 42+ messages in thread
* Re: [PATCH] ARM: Fix zImage file size not aligned with CONFIG_EFI_STUB enabled @ 2017-10-24 8:09 ` Ard Biesheuvel 0 siblings, 0 replies; 42+ messages in thread From: Ard Biesheuvel @ 2017-10-24 8:09 UTC (permalink / raw) To: Russell King - ARM Linux Cc: jeffy, Ingo Molnar, Chris Zhong, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org On 23 October 2017 at 12:45, Russell King - ARM Linux <linux@armlinux.org.uk> wrote: > On Mon, Oct 23, 2017 at 11:50:46AM +0100, Russell King - ARM Linux wrote: >> On Mon, Oct 23, 2017 at 06:24:12PM +0800, jeffy wrote: >> > Hi Russell, >> > >> > Thanks for your reply. >> > >> > On 10/23/2017 04:50 PM, Russell King - ARM Linux wrote: >> > >>> >> > >>>hmm, right, didn't notice the data is already aligned... >> > >>>so it's indeed caused by the ksym: >> > >>> >> > >>> [ 9] .data PROGBITS 006ce000 6d6000 000200 00 WA 0 >> > >>>0 4096 >> > >>> [10] ___ksymtab+sort PROGBITS 006ce200 6d6200 000008 00 WA 0 >> > >>>0 4 >> > >>> [11] .bss NOBITS 006ce208 6d6208 00001c 00 WA 0 >> > >>>0 4 >> > >It's earlier - look for __ksymtab_strings. >> > >> > the problem i meet is the appended dtb code found dtb invalid. i thought >> > that is because of unaligned zImage size, but i was wrong... >> >> Hmm, you really ought not to be using the appended dtb code for modern >> systems - the appended dtb system is there for old boot loaders that >> are incapable of dealing with a dtb. As is said in the option's help >> text: >> >> This is meant as a backward compatibility convenience for those >> systems with a bootloader that can't be upgraded to accommodate >> the documented boot protocol using a device tree. >> >> Beware that there is very little in terms of protection against >> this option being confused by leftover garbage in memory that might >> look like a DTB header after a reboot if no actual DTB is appended >> to zImage. Do not leave this option active in a production kernel >> if you don't intend to always append a DTB. Proper passing of the >> location into r2 of a bootloader provided DTB is always preferable >> to this option. >> >> If you rely on it, and you have something that looks like a dtb after >> the image, then things will go wrong, so it's better _not_ to use it >> and to keep it disabled. >> >> That aside, thanks for doing a more in-depth analysis of what is going >> on, which helps to understand /why/ Ard's fix works (whereas before >> it was rather nebulous.) >> >> I wonder whether we ought to tell the linker to discard any unknown >> sections by adding at the bottom: >> >> /DISCARD/ { *(*) } >> >> but I do think we need to document this, specifically that _edata must >> point to the first byte after the binary file, and that the only >> sections after it are allowed to be the .bss and stack sections. > > Short of adding the discard (which I think is itself risky, we've had > problems in the main kernel's vmlinux.lds in this area), I think we > ought to verify the size of the zImage file, so that the build fails > when we generate a zImage which is wrong, rather than producing a > zImage that is incorrect. Maybe something like this? > > 8<===== > From: Russell King <rmk+kernel@armlinux.org.uk> > Subject: [PATCH] ARM: verify size of zImage > > The linker can sometimes add additional sections to the zImage ELF file > which results in the zImage binary being larger than expected. This > causes appended DT blobs to fail. > > Verify that the zImage binary is the expected size, and fail the build > if this is not the case. > > Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk> > --- > arch/arm/boot/Makefile | 6 +++++- > arch/arm/boot/verify_zimage.sh | 21 +++++++++++++++++++++ > 2 files changed, 26 insertions(+), 1 deletion(-) > create mode 100755 arch/arm/boot/verify_zimage.sh > > diff --git a/arch/arm/boot/Makefile b/arch/arm/boot/Makefile > index a3af4dc08c3e..1290875556ae 100644 > --- a/arch/arm/boot/Makefile > +++ b/arch/arm/boot/Makefile > @@ -53,6 +53,10 @@ $(obj)/Image $(obj)/zImage: FORCE > > else > > +quiet_cmd_mkzimage = ZIMAGE $@ > +cmd_mkzimage = $(cmd_objcopy) && $(CONFIG_SHELL) -c \ > + '$(srctree)/$(src)/verify_zimage.sh $< $@ "$(NM)" || { rm -f $@; false; }' > + > $(obj)/xipImage: FORCE > @echo 'Kernel not configured for XIP (CONFIG_XIP_KERNEL!=y)' > @false > @@ -64,7 +68,7 @@ $(obj)/compressed/vmlinux: $(obj)/Image FORCE > $(Q)$(MAKE) $(build)=$(obj)/compressed $@ > > $(obj)/zImage: $(obj)/compressed/vmlinux FORCE > - $(call if_changed,objcopy) > + $(call if_changed,mkzimage) > > endif > > diff --git a/arch/arm/boot/verify_zimage.sh b/arch/arm/boot/verify_zimage.sh > new file mode 100755 > index 000000000000..922a93e61aa7 > --- /dev/null > +++ b/arch/arm/boot/verify_zimage.sh > @@ -0,0 +1,21 @@ > +#!/bin/sh > +set -e > + > +vmlinuz="$1" > +zimage="$2" > +nm="$3" > + > +magic_size=$("$nm" "$vmlinuz" | perl -e 'while (<>) { > + $magic_start = hex($1) if /^([[:xdigit:]]+) . _magic_start$/; > + $magic_end = hex($1) if /^([[:xdigit:]]+) . _magic_end$/; > +}; printf "%d\n", $magic_end - $magic_start;') > + > +zimage_size=$(stat -c '%s' "$zimage") > + > +# Verify that the resulting binary matches the size contained within > +# the binary (iow, the linker has not added any additional sections.) > +if [ $magic_size -ne $zimage_size ]; then > + echo "zImage size ($zimage_size) disagrees with linked size ($magic_size)" >&2 > + exit 1 > +fi > +exit 0 > -- > 2.7.4 > The following patch appears to fix the issue as well: diff --git a/arch/arm/boot/compressed/vmlinux.lds.S b/arch/arm/boot/compressed/vmlinux.lds.S index 7a4c59154361..0e0f504e256e 100644 --- a/arch/arm/boot/compressed/vmlinux.lds.S +++ b/arch/arm/boot/compressed/vmlinux.lds.S @@ -83,7 +83,9 @@ SECTIONS __pecoff_data_rawsize = . - ADDR(.data); #endif - _edata = .; + .edata (NOLOAD) : { + _edata = .; + } _magic_sig = ZIMAGE_MAGIC(0x016f2818); _magic_start = ZIMAGE_MAGIC(_start); because the NOLOAD triggers the linker to emit all PROGBITS sections before _edata, including unknown ones. E.g., in my binary, it gives me 00798020 D __pecoff_data_end 00798200 d __ksymtab_sort 00798208 B __bss_start 00798208 D _edata ^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH] ARM: Fix zImage file size not aligned with CONFIG_EFI_STUB enabled 2017-10-24 8:09 ` Ard Biesheuvel @ 2017-10-24 9:09 ` Russell King - ARM Linux -1 siblings, 0 replies; 42+ messages in thread From: Russell King - ARM Linux @ 2017-10-24 9:09 UTC (permalink / raw) To: linux-arm-kernel On Tue, Oct 24, 2017 at 09:09:52AM +0100, Ard Biesheuvel wrote: > The following patch appears to fix the issue as well: > > diff --git a/arch/arm/boot/compressed/vmlinux.lds.S > b/arch/arm/boot/compressed/vmlinux.lds.S > index 7a4c59154361..0e0f504e256e 100644 > --- a/arch/arm/boot/compressed/vmlinux.lds.S > +++ b/arch/arm/boot/compressed/vmlinux.lds.S > @@ -83,7 +83,9 @@ SECTIONS > __pecoff_data_rawsize = . - ADDR(.data); > #endif > > - _edata = .; > + .edata (NOLOAD) : { > + _edata = .; > + } > > _magic_sig = ZIMAGE_MAGIC(0x016f2818); > _magic_start = ZIMAGE_MAGIC(_start); > > because the NOLOAD triggers the linker to emit all PROGBITS sections > before _edata, including unknown ones. > > E.g., in my binary, it gives me > > 00798020 D __pecoff_data_end > 00798200 d __ksymtab_sort > 00798208 B __bss_start > 00798208 D _edata The question is: do we want to know when additional sections get emitted into the binary? You've already said that for EFI, you need the size of the binary to be a multiple of 512, so I guess the answer to that is "yes". -- RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up According to speedtest.net: 8.21Mbps down 510kbps up ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH] ARM: Fix zImage file size not aligned with CONFIG_EFI_STUB enabled @ 2017-10-24 9:09 ` Russell King - ARM Linux 0 siblings, 0 replies; 42+ messages in thread From: Russell King - ARM Linux @ 2017-10-24 9:09 UTC (permalink / raw) To: Ard Biesheuvel Cc: linux-kernel@vger.kernel.org, jeffy, Ingo Molnar, linux-arm-kernel@lists.infradead.org, Chris Zhong On Tue, Oct 24, 2017 at 09:09:52AM +0100, Ard Biesheuvel wrote: > The following patch appears to fix the issue as well: > > diff --git a/arch/arm/boot/compressed/vmlinux.lds.S > b/arch/arm/boot/compressed/vmlinux.lds.S > index 7a4c59154361..0e0f504e256e 100644 > --- a/arch/arm/boot/compressed/vmlinux.lds.S > +++ b/arch/arm/boot/compressed/vmlinux.lds.S > @@ -83,7 +83,9 @@ SECTIONS > __pecoff_data_rawsize = . - ADDR(.data); > #endif > > - _edata = .; > + .edata (NOLOAD) : { > + _edata = .; > + } > > _magic_sig = ZIMAGE_MAGIC(0x016f2818); > _magic_start = ZIMAGE_MAGIC(_start); > > because the NOLOAD triggers the linker to emit all PROGBITS sections > before _edata, including unknown ones. > > E.g., in my binary, it gives me > > 00798020 D __pecoff_data_end > 00798200 d __ksymtab_sort > 00798208 B __bss_start > 00798208 D _edata The question is: do we want to know when additional sections get emitted into the binary? You've already said that for EFI, you need the size of the binary to be a multiple of 512, so I guess the answer to that is "yes". -- RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up According to speedtest.net: 8.21Mbps down 510kbps up ^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH] ARM: Fix zImage file size not aligned with CONFIG_EFI_STUB enabled 2017-10-24 9:09 ` Russell King - ARM Linux @ 2017-10-24 9:13 ` Ard Biesheuvel -1 siblings, 0 replies; 42+ messages in thread From: Ard Biesheuvel @ 2017-10-24 9:13 UTC (permalink / raw) To: linux-arm-kernel On 24 October 2017 at 10:09, Russell King - ARM Linux <linux@armlinux.org.uk> wrote: > On Tue, Oct 24, 2017 at 09:09:52AM +0100, Ard Biesheuvel wrote: >> The following patch appears to fix the issue as well: >> >> diff --git a/arch/arm/boot/compressed/vmlinux.lds.S >> b/arch/arm/boot/compressed/vmlinux.lds.S >> index 7a4c59154361..0e0f504e256e 100644 >> --- a/arch/arm/boot/compressed/vmlinux.lds.S >> +++ b/arch/arm/boot/compressed/vmlinux.lds.S >> @@ -83,7 +83,9 @@ SECTIONS >> __pecoff_data_rawsize = . - ADDR(.data); >> #endif >> >> - _edata = .; >> + .edata (NOLOAD) : { >> + _edata = .; >> + } >> >> _magic_sig = ZIMAGE_MAGIC(0x016f2818); >> _magic_start = ZIMAGE_MAGIC(_start); >> >> because the NOLOAD triggers the linker to emit all PROGBITS sections >> before _edata, including unknown ones. >> >> E.g., in my binary, it gives me >> >> 00798020 D __pecoff_data_end >> 00798200 d __ksymtab_sort >> 00798208 B __bss_start >> 00798208 D _edata > > The question is: do we want to know when additional sections get > emitted into the binary? > Well, we need to know whether the size of zImage is a multiple of 512 bytes. We could check that separately by adding the following as well #ifdef CONFIG_EFI_STUB ASSERT((_edata % 512 == 0), "zImage file size is not a multiple of 512 bytes") #endif > You've already said that for EFI, you need the size of the binary > to be a multiple of 512, so I guess the answer to that is "yes". > Indeed. ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH] ARM: Fix zImage file size not aligned with CONFIG_EFI_STUB enabled @ 2017-10-24 9:13 ` Ard Biesheuvel 0 siblings, 0 replies; 42+ messages in thread From: Ard Biesheuvel @ 2017-10-24 9:13 UTC (permalink / raw) To: Russell King - ARM Linux Cc: linux-kernel@vger.kernel.org, jeffy, Ingo Molnar, linux-arm-kernel@lists.infradead.org, Chris Zhong On 24 October 2017 at 10:09, Russell King - ARM Linux <linux@armlinux.org.uk> wrote: > On Tue, Oct 24, 2017 at 09:09:52AM +0100, Ard Biesheuvel wrote: >> The following patch appears to fix the issue as well: >> >> diff --git a/arch/arm/boot/compressed/vmlinux.lds.S >> b/arch/arm/boot/compressed/vmlinux.lds.S >> index 7a4c59154361..0e0f504e256e 100644 >> --- a/arch/arm/boot/compressed/vmlinux.lds.S >> +++ b/arch/arm/boot/compressed/vmlinux.lds.S >> @@ -83,7 +83,9 @@ SECTIONS >> __pecoff_data_rawsize = . - ADDR(.data); >> #endif >> >> - _edata = .; >> + .edata (NOLOAD) : { >> + _edata = .; >> + } >> >> _magic_sig = ZIMAGE_MAGIC(0x016f2818); >> _magic_start = ZIMAGE_MAGIC(_start); >> >> because the NOLOAD triggers the linker to emit all PROGBITS sections >> before _edata, including unknown ones. >> >> E.g., in my binary, it gives me >> >> 00798020 D __pecoff_data_end >> 00798200 d __ksymtab_sort >> 00798208 B __bss_start >> 00798208 D _edata > > The question is: do we want to know when additional sections get > emitted into the binary? > Well, we need to know whether the size of zImage is a multiple of 512 bytes. We could check that separately by adding the following as well #ifdef CONFIG_EFI_STUB ASSERT((_edata % 512 == 0), "zImage file size is not a multiple of 512 bytes") #endif > You've already said that for EFI, you need the size of the binary > to be a multiple of 512, so I guess the answer to that is "yes". > Indeed. ^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH] ARM: Fix zImage file size not aligned with CONFIG_EFI_STUB enabled 2017-10-24 9:13 ` Ard Biesheuvel @ 2017-10-24 9:22 ` Russell King - ARM Linux -1 siblings, 0 replies; 42+ messages in thread From: Russell King - ARM Linux @ 2017-10-24 9:22 UTC (permalink / raw) To: linux-arm-kernel On Tue, Oct 24, 2017 at 10:13:09AM +0100, Ard Biesheuvel wrote: > On 24 October 2017 at 10:09, Russell King - ARM Linux > <linux@armlinux.org.uk> wrote: > > The question is: do we want to know when additional sections get > > emitted into the binary? > > Well, we need to know whether the size of zImage is a multiple of 512 > bytes. We could check that separately by adding the following as well > > #ifdef CONFIG_EFI_STUB > ASSERT((_edata % 512 == 0), "zImage file size is not a multiple of 512 bytes") ASSERT((_edata - _text) % 512 == 0, "EFI zImage file size is not a multiple of 512 bytes") This would only catch them when EFI is enabled, which doesn't give very good build coverage. I still prefer my solution over adding the assert and a section for _edata - my solution catches it with EFI disabled as well. A variant on that would be this, which should catch additional sections for EFI and non-EFI: diff --git a/arch/arm/boot/compressed/vmlinux.lds.S b/arch/arm/boot/compressed/vmlinux.lds.S index b38dcef90756..9d0c5d80979a 100644 --- a/arch/arm/boot/compressed/vmlinux.lds.S +++ b/arch/arm/boot/compressed/vmlinux.lds.S @@ -95,6 +95,10 @@ SECTIONS _edata = .; + .image_end (NOLOAD) : { + _image_end = .; + } + _magic_sig = ZIMAGE_MAGIC(0x016f2818); _magic_start = ZIMAGE_MAGIC(_start); _magic_end = ZIMAGE_MAGIC(_edata); @@ -119,3 +123,5 @@ SECTIONS .stab.indexstr 0 : { *(.stab.indexstr) } .comment 0 : { *(.comment) } } + +ASSERT(image_end == end, "zImage file size is incorrect") -- RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up According to speedtest.net: 8.21Mbps down 510kbps up ^ permalink raw reply related [flat|nested] 42+ messages in thread
* Re: [PATCH] ARM: Fix zImage file size not aligned with CONFIG_EFI_STUB enabled @ 2017-10-24 9:22 ` Russell King - ARM Linux 0 siblings, 0 replies; 42+ messages in thread From: Russell King - ARM Linux @ 2017-10-24 9:22 UTC (permalink / raw) To: Ard Biesheuvel Cc: Chris Zhong, jeffy, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, Ingo Molnar On Tue, Oct 24, 2017 at 10:13:09AM +0100, Ard Biesheuvel wrote: > On 24 October 2017 at 10:09, Russell King - ARM Linux > <linux@armlinux.org.uk> wrote: > > The question is: do we want to know when additional sections get > > emitted into the binary? > > Well, we need to know whether the size of zImage is a multiple of 512 > bytes. We could check that separately by adding the following as well > > #ifdef CONFIG_EFI_STUB > ASSERT((_edata % 512 == 0), "zImage file size is not a multiple of 512 bytes") ASSERT((_edata - _text) % 512 == 0, "EFI zImage file size is not a multiple of 512 bytes") This would only catch them when EFI is enabled, which doesn't give very good build coverage. I still prefer my solution over adding the assert and a section for _edata - my solution catches it with EFI disabled as well. A variant on that would be this, which should catch additional sections for EFI and non-EFI: diff --git a/arch/arm/boot/compressed/vmlinux.lds.S b/arch/arm/boot/compressed/vmlinux.lds.S index b38dcef90756..9d0c5d80979a 100644 --- a/arch/arm/boot/compressed/vmlinux.lds.S +++ b/arch/arm/boot/compressed/vmlinux.lds.S @@ -95,6 +95,10 @@ SECTIONS _edata = .; + .image_end (NOLOAD) : { + _image_end = .; + } + _magic_sig = ZIMAGE_MAGIC(0x016f2818); _magic_start = ZIMAGE_MAGIC(_start); _magic_end = ZIMAGE_MAGIC(_edata); @@ -119,3 +123,5 @@ SECTIONS .stab.indexstr 0 : { *(.stab.indexstr) } .comment 0 : { *(.comment) } } + +ASSERT(image_end == end, "zImage file size is incorrect") -- RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up According to speedtest.net: 8.21Mbps down 510kbps up ^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH] ARM: Fix zImage file size not aligned with CONFIG_EFI_STUB enabled 2017-10-24 9:22 ` Russell King - ARM Linux @ 2017-10-24 9:26 ` Ard Biesheuvel -1 siblings, 0 replies; 42+ messages in thread From: Ard Biesheuvel @ 2017-10-24 9:26 UTC (permalink / raw) To: linux-arm-kernel On 24 October 2017 at 10:22, Russell King - ARM Linux <linux@armlinux.org.uk> wrote: > On Tue, Oct 24, 2017 at 10:13:09AM +0100, Ard Biesheuvel wrote: >> On 24 October 2017 at 10:09, Russell King - ARM Linux >> <linux@armlinux.org.uk> wrote: >> > The question is: do we want to know when additional sections get >> > emitted into the binary? >> >> Well, we need to know whether the size of zImage is a multiple of 512 >> bytes. We could check that separately by adding the following as well >> >> #ifdef CONFIG_EFI_STUB >> ASSERT((_edata % 512 == 0), "zImage file size is not a multiple of 512 bytes") > > ASSERT((_edata - _text) % 512 == 0, "EFI zImage file size is not a multiple of 512 bytes") > > This would only catch them when EFI is enabled, which doesn't give very > good build coverage. I still prefer my solution over adding the assert > and a section for _edata - my solution catches it with EFI disabled as > well. > > A variant on that would be this, which should catch additional sections > for EFI and non-EFI: > > diff --git a/arch/arm/boot/compressed/vmlinux.lds.S b/arch/arm/boot/compressed/vmlinux.lds.S > index b38dcef90756..9d0c5d80979a 100644 > --- a/arch/arm/boot/compressed/vmlinux.lds.S > +++ b/arch/arm/boot/compressed/vmlinux.lds.S > @@ -95,6 +95,10 @@ SECTIONS > > _edata = .; > > + .image_end (NOLOAD) : { > + _image_end = .; > + } > + > _magic_sig = ZIMAGE_MAGIC(0x016f2818); > _magic_start = ZIMAGE_MAGIC(_start); > _magic_end = ZIMAGE_MAGIC(_edata); > @@ -119,3 +123,5 @@ SECTIONS > .stab.indexstr 0 : { *(.stab.indexstr) } > .comment 0 : { *(.comment) } > } > + > +ASSERT(image_end == end, "zImage file size is incorrect") > I take it you mean _image_end == _edata here? That works for me: Tested-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> (with CONFIG_EFI enabled) ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH] ARM: Fix zImage file size not aligned with CONFIG_EFI_STUB enabled @ 2017-10-24 9:26 ` Ard Biesheuvel 0 siblings, 0 replies; 42+ messages in thread From: Ard Biesheuvel @ 2017-10-24 9:26 UTC (permalink / raw) To: Russell King - ARM Linux Cc: Chris Zhong, jeffy, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, Ingo Molnar On 24 October 2017 at 10:22, Russell King - ARM Linux <linux@armlinux.org.uk> wrote: > On Tue, Oct 24, 2017 at 10:13:09AM +0100, Ard Biesheuvel wrote: >> On 24 October 2017 at 10:09, Russell King - ARM Linux >> <linux@armlinux.org.uk> wrote: >> > The question is: do we want to know when additional sections get >> > emitted into the binary? >> >> Well, we need to know whether the size of zImage is a multiple of 512 >> bytes. We could check that separately by adding the following as well >> >> #ifdef CONFIG_EFI_STUB >> ASSERT((_edata % 512 == 0), "zImage file size is not a multiple of 512 bytes") > > ASSERT((_edata - _text) % 512 == 0, "EFI zImage file size is not a multiple of 512 bytes") > > This would only catch them when EFI is enabled, which doesn't give very > good build coverage. I still prefer my solution over adding the assert > and a section for _edata - my solution catches it with EFI disabled as > well. > > A variant on that would be this, which should catch additional sections > for EFI and non-EFI: > > diff --git a/arch/arm/boot/compressed/vmlinux.lds.S b/arch/arm/boot/compressed/vmlinux.lds.S > index b38dcef90756..9d0c5d80979a 100644 > --- a/arch/arm/boot/compressed/vmlinux.lds.S > +++ b/arch/arm/boot/compressed/vmlinux.lds.S > @@ -95,6 +95,10 @@ SECTIONS > > _edata = .; > > + .image_end (NOLOAD) : { > + _image_end = .; > + } > + > _magic_sig = ZIMAGE_MAGIC(0x016f2818); > _magic_start = ZIMAGE_MAGIC(_start); > _magic_end = ZIMAGE_MAGIC(_edata); > @@ -119,3 +123,5 @@ SECTIONS > .stab.indexstr 0 : { *(.stab.indexstr) } > .comment 0 : { *(.comment) } > } > + > +ASSERT(image_end == end, "zImage file size is incorrect") > I take it you mean _image_end == _edata here? That works for me: Tested-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> (with CONFIG_EFI enabled) ^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH] ARM: Fix zImage file size not aligned with CONFIG_EFI_STUB enabled 2017-10-24 9:26 ` Ard Biesheuvel @ 2017-10-24 9:30 ` Ard Biesheuvel -1 siblings, 0 replies; 42+ messages in thread From: Ard Biesheuvel @ 2017-10-24 9:30 UTC (permalink / raw) To: linux-arm-kernel On 24 October 2017 at 10:26, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote: > On 24 October 2017 at 10:22, Russell King - ARM Linux > <linux@armlinux.org.uk> wrote: >> On Tue, Oct 24, 2017 at 10:13:09AM +0100, Ard Biesheuvel wrote: >>> On 24 October 2017 at 10:09, Russell King - ARM Linux >>> <linux@armlinux.org.uk> wrote: >>> > The question is: do we want to know when additional sections get >>> > emitted into the binary? >>> >>> Well, we need to know whether the size of zImage is a multiple of 512 >>> bytes. We could check that separately by adding the following as well >>> >>> #ifdef CONFIG_EFI_STUB >>> ASSERT((_edata % 512 == 0), "zImage file size is not a multiple of 512 bytes") >> >> ASSERT((_edata - _text) % 512 == 0, "EFI zImage file size is not a multiple of 512 bytes") >> >> This would only catch them when EFI is enabled, which doesn't give very >> good build coverage. I still prefer my solution over adding the assert >> and a section for _edata - my solution catches it with EFI disabled as >> well. >> >> A variant on that would be this, which should catch additional sections >> for EFI and non-EFI: >> >> diff --git a/arch/arm/boot/compressed/vmlinux.lds.S b/arch/arm/boot/compressed/vmlinux.lds.S >> index b38dcef90756..9d0c5d80979a 100644 >> --- a/arch/arm/boot/compressed/vmlinux.lds.S >> +++ b/arch/arm/boot/compressed/vmlinux.lds.S >> @@ -95,6 +95,10 @@ SECTIONS >> >> _edata = .; >> >> + .image_end (NOLOAD) : { >> + _image_end = .; >> + } >> + >> _magic_sig = ZIMAGE_MAGIC(0x016f2818); >> _magic_start = ZIMAGE_MAGIC(_start); >> _magic_end = ZIMAGE_MAGIC(_edata); >> @@ -119,3 +123,5 @@ SECTIONS >> .stab.indexstr 0 : { *(.stab.indexstr) } >> .comment 0 : { *(.comment) } >> } >> + >> +ASSERT(image_end == end, "zImage file size is incorrect") >> > > I take it you mean > > _image_end == _edata > > here? That works for me: > > Tested-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > > (with CONFIG_EFI enabled) BTW you could simplify that to diff --git a/arch/arm/boot/compressed/vmlinux.lds.S b/arch/arm/boot/compressed/vmlinux.lds.S index 7a4c59154361..204d4580a04f 100644 --- a/arch/arm/boot/compressed/vmlinux.lds.S +++ b/arch/arm/boot/compressed/vmlinux.lds.S @@ -85,6 +85,10 @@ SECTIONS _edata = .; + .image_end (NOLOAD) : { + ASSERT(. == _edata, "zImage file size is incorrect"); + } + _magic_sig = ZIMAGE_MAGIC(0x016f2818); _magic_start = ZIMAGE_MAGIC(_start); _magic_end = ZIMAGE_MAGIC(_edata); ^ permalink raw reply related [flat|nested] 42+ messages in thread
* Re: [PATCH] ARM: Fix zImage file size not aligned with CONFIG_EFI_STUB enabled @ 2017-10-24 9:30 ` Ard Biesheuvel 0 siblings, 0 replies; 42+ messages in thread From: Ard Biesheuvel @ 2017-10-24 9:30 UTC (permalink / raw) To: Russell King - ARM Linux Cc: Chris Zhong, jeffy, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, Ingo Molnar On 24 October 2017 at 10:26, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote: > On 24 October 2017 at 10:22, Russell King - ARM Linux > <linux@armlinux.org.uk> wrote: >> On Tue, Oct 24, 2017 at 10:13:09AM +0100, Ard Biesheuvel wrote: >>> On 24 October 2017 at 10:09, Russell King - ARM Linux >>> <linux@armlinux.org.uk> wrote: >>> > The question is: do we want to know when additional sections get >>> > emitted into the binary? >>> >>> Well, we need to know whether the size of zImage is a multiple of 512 >>> bytes. We could check that separately by adding the following as well >>> >>> #ifdef CONFIG_EFI_STUB >>> ASSERT((_edata % 512 == 0), "zImage file size is not a multiple of 512 bytes") >> >> ASSERT((_edata - _text) % 512 == 0, "EFI zImage file size is not a multiple of 512 bytes") >> >> This would only catch them when EFI is enabled, which doesn't give very >> good build coverage. I still prefer my solution over adding the assert >> and a section for _edata - my solution catches it with EFI disabled as >> well. >> >> A variant on that would be this, which should catch additional sections >> for EFI and non-EFI: >> >> diff --git a/arch/arm/boot/compressed/vmlinux.lds.S b/arch/arm/boot/compressed/vmlinux.lds.S >> index b38dcef90756..9d0c5d80979a 100644 >> --- a/arch/arm/boot/compressed/vmlinux.lds.S >> +++ b/arch/arm/boot/compressed/vmlinux.lds.S >> @@ -95,6 +95,10 @@ SECTIONS >> >> _edata = .; >> >> + .image_end (NOLOAD) : { >> + _image_end = .; >> + } >> + >> _magic_sig = ZIMAGE_MAGIC(0x016f2818); >> _magic_start = ZIMAGE_MAGIC(_start); >> _magic_end = ZIMAGE_MAGIC(_edata); >> @@ -119,3 +123,5 @@ SECTIONS >> .stab.indexstr 0 : { *(.stab.indexstr) } >> .comment 0 : { *(.comment) } >> } >> + >> +ASSERT(image_end == end, "zImage file size is incorrect") >> > > I take it you mean > > _image_end == _edata > > here? That works for me: > > Tested-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > > (with CONFIG_EFI enabled) BTW you could simplify that to diff --git a/arch/arm/boot/compressed/vmlinux.lds.S b/arch/arm/boot/compressed/vmlinux.lds.S index 7a4c59154361..204d4580a04f 100644 --- a/arch/arm/boot/compressed/vmlinux.lds.S +++ b/arch/arm/boot/compressed/vmlinux.lds.S @@ -85,6 +85,10 @@ SECTIONS _edata = .; + .image_end (NOLOAD) : { + ASSERT(. == _edata, "zImage file size is incorrect"); + } + _magic_sig = ZIMAGE_MAGIC(0x016f2818); _magic_start = ZIMAGE_MAGIC(_start); _magic_end = ZIMAGE_MAGIC(_edata); ^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH] ARM: Fix zImage file size not aligned with CONFIG_EFI_STUB enabled 2017-10-24 9:30 ` Ard Biesheuvel @ 2017-10-24 9:38 ` Russell King - ARM Linux -1 siblings, 0 replies; 42+ messages in thread From: Russell King - ARM Linux @ 2017-10-24 9:38 UTC (permalink / raw) To: linux-arm-kernel On Tue, Oct 24, 2017 at 10:30:41AM +0100, Ard Biesheuvel wrote: > On 24 October 2017 at 10:26, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote: > > On 24 October 2017 at 10:22, Russell King - ARM Linux > > <linux@armlinux.org.uk> wrote: > >> On Tue, Oct 24, 2017 at 10:13:09AM +0100, Ard Biesheuvel wrote: > >>> On 24 October 2017 at 10:09, Russell King - ARM Linux > >>> <linux@armlinux.org.uk> wrote: > >>> > The question is: do we want to know when additional sections get > >>> > emitted into the binary? > >>> > >>> Well, we need to know whether the size of zImage is a multiple of 512 > >>> bytes. We could check that separately by adding the following as well > >>> > >>> #ifdef CONFIG_EFI_STUB > >>> ASSERT((_edata % 512 == 0), "zImage file size is not a multiple of 512 bytes") > >> > >> ASSERT((_edata - _text) % 512 == 0, "EFI zImage file size is not a multiple of 512 bytes") > >> > >> This would only catch them when EFI is enabled, which doesn't give very > >> good build coverage. I still prefer my solution over adding the assert > >> and a section for _edata - my solution catches it with EFI disabled as > >> well. > >> > >> A variant on that would be this, which should catch additional sections > >> for EFI and non-EFI: > >> > >> diff --git a/arch/arm/boot/compressed/vmlinux.lds.S b/arch/arm/boot/compressed/vmlinux.lds.S > >> index b38dcef90756..9d0c5d80979a 100644 > >> --- a/arch/arm/boot/compressed/vmlinux.lds.S > >> +++ b/arch/arm/boot/compressed/vmlinux.lds.S > >> @@ -95,6 +95,10 @@ SECTIONS > >> > >> _edata = .; > >> > >> + .image_end (NOLOAD) : { > >> + _image_end = .; > >> + } > >> + > >> _magic_sig = ZIMAGE_MAGIC(0x016f2818); > >> _magic_start = ZIMAGE_MAGIC(_start); > >> _magic_end = ZIMAGE_MAGIC(_edata); > >> @@ -119,3 +123,5 @@ SECTIONS > >> .stab.indexstr 0 : { *(.stab.indexstr) } > >> .comment 0 : { *(.comment) } > >> } > >> + > >> +ASSERT(image_end == end, "zImage file size is incorrect") > >> > > > > I take it you mean > > > > _image_end == _edata > > > > here? That works for me: > > > > Tested-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > > > > (with CONFIG_EFI enabled) > > > BTW you could simplify that to > > diff --git a/arch/arm/boot/compressed/vmlinux.lds.S > b/arch/arm/boot/compressed/vmlinux.lds.S > index 7a4c59154361..204d4580a04f 100644 > --- a/arch/arm/boot/compressed/vmlinux.lds.S > +++ b/arch/arm/boot/compressed/vmlinux.lds.S > @@ -85,6 +85,10 @@ SECTIONS > > _edata = .; > > + .image_end (NOLOAD) : { > + ASSERT(. == _edata, "zImage file size is incorrect"); > + } Yep. Okay, so now we have a patch from Arnd to fix the alignment problem (so don't need alignment of the piggydata), and we have a patch to catch the additional sections. That leaves a patch to sort out the additional sections. Do you have any preference - I'd prefer one that I can merge along with these changes? One way forward would be to temporarily add the /DISCARD/ for the ksym sections, which can then be removed once your sort() removal gets added to the EFI trees. -- RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up According to speedtest.net: 8.21Mbps down 510kbps up ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH] ARM: Fix zImage file size not aligned with CONFIG_EFI_STUB enabled @ 2017-10-24 9:38 ` Russell King - ARM Linux 0 siblings, 0 replies; 42+ messages in thread From: Russell King - ARM Linux @ 2017-10-24 9:38 UTC (permalink / raw) To: Ard Biesheuvel Cc: Chris Zhong, jeffy, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, Ingo Molnar On Tue, Oct 24, 2017 at 10:30:41AM +0100, Ard Biesheuvel wrote: > On 24 October 2017 at 10:26, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote: > > On 24 October 2017 at 10:22, Russell King - ARM Linux > > <linux@armlinux.org.uk> wrote: > >> On Tue, Oct 24, 2017 at 10:13:09AM +0100, Ard Biesheuvel wrote: > >>> On 24 October 2017 at 10:09, Russell King - ARM Linux > >>> <linux@armlinux.org.uk> wrote: > >>> > The question is: do we want to know when additional sections get > >>> > emitted into the binary? > >>> > >>> Well, we need to know whether the size of zImage is a multiple of 512 > >>> bytes. We could check that separately by adding the following as well > >>> > >>> #ifdef CONFIG_EFI_STUB > >>> ASSERT((_edata % 512 == 0), "zImage file size is not a multiple of 512 bytes") > >> > >> ASSERT((_edata - _text) % 512 == 0, "EFI zImage file size is not a multiple of 512 bytes") > >> > >> This would only catch them when EFI is enabled, which doesn't give very > >> good build coverage. I still prefer my solution over adding the assert > >> and a section for _edata - my solution catches it with EFI disabled as > >> well. > >> > >> A variant on that would be this, which should catch additional sections > >> for EFI and non-EFI: > >> > >> diff --git a/arch/arm/boot/compressed/vmlinux.lds.S b/arch/arm/boot/compressed/vmlinux.lds.S > >> index b38dcef90756..9d0c5d80979a 100644 > >> --- a/arch/arm/boot/compressed/vmlinux.lds.S > >> +++ b/arch/arm/boot/compressed/vmlinux.lds.S > >> @@ -95,6 +95,10 @@ SECTIONS > >> > >> _edata = .; > >> > >> + .image_end (NOLOAD) : { > >> + _image_end = .; > >> + } > >> + > >> _magic_sig = ZIMAGE_MAGIC(0x016f2818); > >> _magic_start = ZIMAGE_MAGIC(_start); > >> _magic_end = ZIMAGE_MAGIC(_edata); > >> @@ -119,3 +123,5 @@ SECTIONS > >> .stab.indexstr 0 : { *(.stab.indexstr) } > >> .comment 0 : { *(.comment) } > >> } > >> + > >> +ASSERT(image_end == end, "zImage file size is incorrect") > >> > > > > I take it you mean > > > > _image_end == _edata > > > > here? That works for me: > > > > Tested-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > > > > (with CONFIG_EFI enabled) > > > BTW you could simplify that to > > diff --git a/arch/arm/boot/compressed/vmlinux.lds.S > b/arch/arm/boot/compressed/vmlinux.lds.S > index 7a4c59154361..204d4580a04f 100644 > --- a/arch/arm/boot/compressed/vmlinux.lds.S > +++ b/arch/arm/boot/compressed/vmlinux.lds.S > @@ -85,6 +85,10 @@ SECTIONS > > _edata = .; > > + .image_end (NOLOAD) : { > + ASSERT(. == _edata, "zImage file size is incorrect"); > + } Yep. Okay, so now we have a patch from Arnd to fix the alignment problem (so don't need alignment of the piggydata), and we have a patch to catch the additional sections. That leaves a patch to sort out the additional sections. Do you have any preference - I'd prefer one that I can merge along with these changes? One way forward would be to temporarily add the /DISCARD/ for the ksym sections, which can then be removed once your sort() removal gets added to the EFI trees. -- RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up According to speedtest.net: 8.21Mbps down 510kbps up ^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH] ARM: Fix zImage file size not aligned with CONFIG_EFI_STUB enabled 2017-10-24 9:38 ` Russell King - ARM Linux @ 2017-10-24 9:44 ` Ard Biesheuvel -1 siblings, 0 replies; 42+ messages in thread From: Ard Biesheuvel @ 2017-10-24 9:44 UTC (permalink / raw) To: linux-arm-kernel On 24 October 2017 at 10:38, Russell King - ARM Linux <linux@armlinux.org.uk> wrote: > On Tue, Oct 24, 2017 at 10:30:41AM +0100, Ard Biesheuvel wrote: >> On 24 October 2017 at 10:26, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote: >> > On 24 October 2017 at 10:22, Russell King - ARM Linux >> > <linux@armlinux.org.uk> wrote: >> >> On Tue, Oct 24, 2017 at 10:13:09AM +0100, Ard Biesheuvel wrote: >> >>> On 24 October 2017 at 10:09, Russell King - ARM Linux >> >>> <linux@armlinux.org.uk> wrote: >> >>> > The question is: do we want to know when additional sections get >> >>> > emitted into the binary? >> >>> >> >>> Well, we need to know whether the size of zImage is a multiple of 512 >> >>> bytes. We could check that separately by adding the following as well >> >>> >> >>> #ifdef CONFIG_EFI_STUB >> >>> ASSERT((_edata % 512 == 0), "zImage file size is not a multiple of 512 bytes") >> >> >> >> ASSERT((_edata - _text) % 512 == 0, "EFI zImage file size is not a multiple of 512 bytes") >> >> >> >> This would only catch them when EFI is enabled, which doesn't give very >> >> good build coverage. I still prefer my solution over adding the assert >> >> and a section for _edata - my solution catches it with EFI disabled as >> >> well. >> >> >> >> A variant on that would be this, which should catch additional sections >> >> for EFI and non-EFI: >> >> >> >> diff --git a/arch/arm/boot/compressed/vmlinux.lds.S b/arch/arm/boot/compressed/vmlinux.lds.S >> >> index b38dcef90756..9d0c5d80979a 100644 >> >> --- a/arch/arm/boot/compressed/vmlinux.lds.S >> >> +++ b/arch/arm/boot/compressed/vmlinux.lds.S >> >> @@ -95,6 +95,10 @@ SECTIONS >> >> >> >> _edata = .; >> >> >> >> + .image_end (NOLOAD) : { >> >> + _image_end = .; >> >> + } >> >> + >> >> _magic_sig = ZIMAGE_MAGIC(0x016f2818); >> >> _magic_start = ZIMAGE_MAGIC(_start); >> >> _magic_end = ZIMAGE_MAGIC(_edata); >> >> @@ -119,3 +123,5 @@ SECTIONS >> >> .stab.indexstr 0 : { *(.stab.indexstr) } >> >> .comment 0 : { *(.comment) } >> >> } >> >> + >> >> +ASSERT(image_end == end, "zImage file size is incorrect") >> >> >> > >> > I take it you mean >> > >> > _image_end == _edata >> > >> > here? That works for me: >> > >> > Tested-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> >> > >> > (with CONFIG_EFI enabled) >> >> >> BTW you could simplify that to >> >> diff --git a/arch/arm/boot/compressed/vmlinux.lds.S >> b/arch/arm/boot/compressed/vmlinux.lds.S >> index 7a4c59154361..204d4580a04f 100644 >> --- a/arch/arm/boot/compressed/vmlinux.lds.S >> +++ b/arch/arm/boot/compressed/vmlinux.lds.S >> @@ -85,6 +85,10 @@ SECTIONS >> >> _edata = .; >> >> + .image_end (NOLOAD) : { >> + ASSERT(. == _edata, "zImage file size is incorrect"); >> + } > > Yep. Okay, so now we have a patch from Arnd to fix the alignment problem > (so don't need alignment of the piggydata), and we have a patch to catch > the additional sections. That leaves a patch to sort out the additional > sections. > > Do you have any preference - I'd prefer one that I can merge along with > these changes? One way forward would be to temporarily add the /DISCARD/ > for the ksym sections, which can then be removed once your sort() removal > gets added to the EFI trees. > Well, I can respin that patch to only discard the ksymtab/kcrctab sections (and drop the alignment of piggydata), but I'd prefer to make it permanent if you don't mind. I still intend to remove the sort() call, given that ARM doesn't need it, but it seems more future proof to me to always discard those sections, and if we end up incorporating more core kernel code into the EFI stub, we won't need to revisit this (although I am not aware of any reasons why we would be doing that in the near future) ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH] ARM: Fix zImage file size not aligned with CONFIG_EFI_STUB enabled @ 2017-10-24 9:44 ` Ard Biesheuvel 0 siblings, 0 replies; 42+ messages in thread From: Ard Biesheuvel @ 2017-10-24 9:44 UTC (permalink / raw) To: Russell King - ARM Linux Cc: Chris Zhong, jeffy, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, Ingo Molnar On 24 October 2017 at 10:38, Russell King - ARM Linux <linux@armlinux.org.uk> wrote: > On Tue, Oct 24, 2017 at 10:30:41AM +0100, Ard Biesheuvel wrote: >> On 24 October 2017 at 10:26, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote: >> > On 24 October 2017 at 10:22, Russell King - ARM Linux >> > <linux@armlinux.org.uk> wrote: >> >> On Tue, Oct 24, 2017 at 10:13:09AM +0100, Ard Biesheuvel wrote: >> >>> On 24 October 2017 at 10:09, Russell King - ARM Linux >> >>> <linux@armlinux.org.uk> wrote: >> >>> > The question is: do we want to know when additional sections get >> >>> > emitted into the binary? >> >>> >> >>> Well, we need to know whether the size of zImage is a multiple of 512 >> >>> bytes. We could check that separately by adding the following as well >> >>> >> >>> #ifdef CONFIG_EFI_STUB >> >>> ASSERT((_edata % 512 == 0), "zImage file size is not a multiple of 512 bytes") >> >> >> >> ASSERT((_edata - _text) % 512 == 0, "EFI zImage file size is not a multiple of 512 bytes") >> >> >> >> This would only catch them when EFI is enabled, which doesn't give very >> >> good build coverage. I still prefer my solution over adding the assert >> >> and a section for _edata - my solution catches it with EFI disabled as >> >> well. >> >> >> >> A variant on that would be this, which should catch additional sections >> >> for EFI and non-EFI: >> >> >> >> diff --git a/arch/arm/boot/compressed/vmlinux.lds.S b/arch/arm/boot/compressed/vmlinux.lds.S >> >> index b38dcef90756..9d0c5d80979a 100644 >> >> --- a/arch/arm/boot/compressed/vmlinux.lds.S >> >> +++ b/arch/arm/boot/compressed/vmlinux.lds.S >> >> @@ -95,6 +95,10 @@ SECTIONS >> >> >> >> _edata = .; >> >> >> >> + .image_end (NOLOAD) : { >> >> + _image_end = .; >> >> + } >> >> + >> >> _magic_sig = ZIMAGE_MAGIC(0x016f2818); >> >> _magic_start = ZIMAGE_MAGIC(_start); >> >> _magic_end = ZIMAGE_MAGIC(_edata); >> >> @@ -119,3 +123,5 @@ SECTIONS >> >> .stab.indexstr 0 : { *(.stab.indexstr) } >> >> .comment 0 : { *(.comment) } >> >> } >> >> + >> >> +ASSERT(image_end == end, "zImage file size is incorrect") >> >> >> > >> > I take it you mean >> > >> > _image_end == _edata >> > >> > here? That works for me: >> > >> > Tested-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> >> > >> > (with CONFIG_EFI enabled) >> >> >> BTW you could simplify that to >> >> diff --git a/arch/arm/boot/compressed/vmlinux.lds.S >> b/arch/arm/boot/compressed/vmlinux.lds.S >> index 7a4c59154361..204d4580a04f 100644 >> --- a/arch/arm/boot/compressed/vmlinux.lds.S >> +++ b/arch/arm/boot/compressed/vmlinux.lds.S >> @@ -85,6 +85,10 @@ SECTIONS >> >> _edata = .; >> >> + .image_end (NOLOAD) : { >> + ASSERT(. == _edata, "zImage file size is incorrect"); >> + } > > Yep. Okay, so now we have a patch from Arnd to fix the alignment problem > (so don't need alignment of the piggydata), and we have a patch to catch > the additional sections. That leaves a patch to sort out the additional > sections. > > Do you have any preference - I'd prefer one that I can merge along with > these changes? One way forward would be to temporarily add the /DISCARD/ > for the ksym sections, which can then be removed once your sort() removal > gets added to the EFI trees. > Well, I can respin that patch to only discard the ksymtab/kcrctab sections (and drop the alignment of piggydata), but I'd prefer to make it permanent if you don't mind. I still intend to remove the sort() call, given that ARM doesn't need it, but it seems more future proof to me to always discard those sections, and if we end up incorporating more core kernel code into the EFI stub, we won't need to revisit this (although I am not aware of any reasons why we would be doing that in the near future) ^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH] ARM: Fix zImage file size not aligned with CONFIG_EFI_STUB enabled 2017-10-24 9:44 ` Ard Biesheuvel @ 2017-10-24 9:54 ` Russell King - ARM Linux -1 siblings, 0 replies; 42+ messages in thread From: Russell King - ARM Linux @ 2017-10-24 9:54 UTC (permalink / raw) To: linux-arm-kernel On Tue, Oct 24, 2017 at 10:44:00AM +0100, Ard Biesheuvel wrote: > On 24 October 2017 at 10:38, Russell King - ARM Linux > <linux@armlinux.org.uk> wrote: > > Do you have any preference - I'd prefer one that I can merge along with > > these changes? One way forward would be to temporarily add the /DISCARD/ > > for the ksym sections, which can then be removed once your sort() removal > > gets added to the EFI trees. > > > > Well, I can respin that patch to only discard the ksymtab/kcrctab > sections (and drop the alignment of piggydata), but I'd prefer to make > it permanent if you don't mind. I still intend to remove the sort() > call, given that ARM doesn't need it, but it seems more future proof > to me to always discard those sections, and if we end up incorporating > more core kernel code into the EFI stub, we won't need to revisit this > (although I am not aware of any reasons why we would be doing that in > the near future) I don't think it's future-proof. The decompressor is a particularly special environment due to its runtime relocation support, where we (eg) don't really support initialised pointers in the .data section. Stuffing code from other parts of the kernel into the decompressor isn't going to work reliably, and I'd much rather discourage the practice. Hence, there's really no reason to keep that discard, and I'd much rather have the build fail if the sections re-appear as a pointer towards something not being correct. A solution to this would be to link the decompressor ELF with -r and wrap that up in a mini-linker, but I suspect that will need quite a re-write because of the self-relocation that the image does if it detects overlap between itself and the destination for the decompressed kernel image. -- RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up According to speedtest.net: 8.21Mbps down 510kbps up ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH] ARM: Fix zImage file size not aligned with CONFIG_EFI_STUB enabled @ 2017-10-24 9:54 ` Russell King - ARM Linux 0 siblings, 0 replies; 42+ messages in thread From: Russell King - ARM Linux @ 2017-10-24 9:54 UTC (permalink / raw) To: Ard Biesheuvel Cc: Chris Zhong, jeffy, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, Ingo Molnar On Tue, Oct 24, 2017 at 10:44:00AM +0100, Ard Biesheuvel wrote: > On 24 October 2017 at 10:38, Russell King - ARM Linux > <linux@armlinux.org.uk> wrote: > > Do you have any preference - I'd prefer one that I can merge along with > > these changes? One way forward would be to temporarily add the /DISCARD/ > > for the ksym sections, which can then be removed once your sort() removal > > gets added to the EFI trees. > > > > Well, I can respin that patch to only discard the ksymtab/kcrctab > sections (and drop the alignment of piggydata), but I'd prefer to make > it permanent if you don't mind. I still intend to remove the sort() > call, given that ARM doesn't need it, but it seems more future proof > to me to always discard those sections, and if we end up incorporating > more core kernel code into the EFI stub, we won't need to revisit this > (although I am not aware of any reasons why we would be doing that in > the near future) I don't think it's future-proof. The decompressor is a particularly special environment due to its runtime relocation support, where we (eg) don't really support initialised pointers in the .data section. Stuffing code from other parts of the kernel into the decompressor isn't going to work reliably, and I'd much rather discourage the practice. Hence, there's really no reason to keep that discard, and I'd much rather have the build fail if the sections re-appear as a pointer towards something not being correct. A solution to this would be to link the decompressor ELF with -r and wrap that up in a mini-linker, but I suspect that will need quite a re-write because of the self-relocation that the image does if it detects overlap between itself and the destination for the decompressed kernel image. -- RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up According to speedtest.net: 8.21Mbps down 510kbps up ^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH] ARM: Fix zImage file size not aligned with CONFIG_EFI_STUB enabled 2017-10-24 9:54 ` Russell King - ARM Linux @ 2017-10-24 10:03 ` Ard Biesheuvel -1 siblings, 0 replies; 42+ messages in thread From: Ard Biesheuvel @ 2017-10-24 10:03 UTC (permalink / raw) To: linux-arm-kernel On 24 October 2017 at 10:54, Russell King - ARM Linux <linux@armlinux.org.uk> wrote: > On Tue, Oct 24, 2017 at 10:44:00AM +0100, Ard Biesheuvel wrote: >> On 24 October 2017 at 10:38, Russell King - ARM Linux >> <linux@armlinux.org.uk> wrote: >> > Do you have any preference - I'd prefer one that I can merge along with >> > these changes? One way forward would be to temporarily add the /DISCARD/ >> > for the ksym sections, which can then be removed once your sort() removal >> > gets added to the EFI trees. >> > >> >> Well, I can respin that patch to only discard the ksymtab/kcrctab >> sections (and drop the alignment of piggydata), but I'd prefer to make >> it permanent if you don't mind. I still intend to remove the sort() >> call, given that ARM doesn't need it, but it seems more future proof >> to me to always discard those sections, and if we end up incorporating >> more core kernel code into the EFI stub, we won't need to revisit this >> (although I am not aware of any reasons why we would be doing that in >> the near future) > > I don't think it's future-proof. The decompressor is a particularly > special environment due to its runtime relocation support, where we > (eg) don't really support initialised pointers in the .data section. > Stuffing code from other parts of the kernel into the decompressor > isn't going to work reliably, and I'd much rather discourage the > practice. > Fair enough. > Hence, there's really no reason to keep that discard, and I'd much > rather have the build fail if the sections re-appear as a pointer > towards something not being correct. > OK. You're welcome to take that sort() removal patch through the ARM tree if you want to keep these patches together. Or I could respin the /discard/ one. Whichever you prefer (although I am leaning towards the former, so we can merge them and be done with it) > A solution to this would be to link the decompressor ELF with -r > and wrap that up in a mini-linker, but I suspect that will need > quite a re-write because of the self-relocation that the image > does if it detects overlap between itself and the destination for > the decompressed kernel image. > It was already quite tricky to get the hybrid PE-COFF/bare metal image working, especially because the stub code is shared between x86, arm64 and ARM, all of which have their own quirks when it comes to the early environment this code executes in. ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH] ARM: Fix zImage file size not aligned with CONFIG_EFI_STUB enabled @ 2017-10-24 10:03 ` Ard Biesheuvel 0 siblings, 0 replies; 42+ messages in thread From: Ard Biesheuvel @ 2017-10-24 10:03 UTC (permalink / raw) To: Russell King - ARM Linux Cc: Chris Zhong, jeffy, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, Ingo Molnar On 24 October 2017 at 10:54, Russell King - ARM Linux <linux@armlinux.org.uk> wrote: > On Tue, Oct 24, 2017 at 10:44:00AM +0100, Ard Biesheuvel wrote: >> On 24 October 2017 at 10:38, Russell King - ARM Linux >> <linux@armlinux.org.uk> wrote: >> > Do you have any preference - I'd prefer one that I can merge along with >> > these changes? One way forward would be to temporarily add the /DISCARD/ >> > for the ksym sections, which can then be removed once your sort() removal >> > gets added to the EFI trees. >> > >> >> Well, I can respin that patch to only discard the ksymtab/kcrctab >> sections (and drop the alignment of piggydata), but I'd prefer to make >> it permanent if you don't mind. I still intend to remove the sort() >> call, given that ARM doesn't need it, but it seems more future proof >> to me to always discard those sections, and if we end up incorporating >> more core kernel code into the EFI stub, we won't need to revisit this >> (although I am not aware of any reasons why we would be doing that in >> the near future) > > I don't think it's future-proof. The decompressor is a particularly > special environment due to its runtime relocation support, where we > (eg) don't really support initialised pointers in the .data section. > Stuffing code from other parts of the kernel into the decompressor > isn't going to work reliably, and I'd much rather discourage the > practice. > Fair enough. > Hence, there's really no reason to keep that discard, and I'd much > rather have the build fail if the sections re-appear as a pointer > towards something not being correct. > OK. You're welcome to take that sort() removal patch through the ARM tree if you want to keep these patches together. Or I could respin the /discard/ one. Whichever you prefer (although I am leaning towards the former, so we can merge them and be done with it) > A solution to this would be to link the decompressor ELF with -r > and wrap that up in a mini-linker, but I suspect that will need > quite a re-write because of the self-relocation that the image > does if it detects overlap between itself and the destination for > the decompressed kernel image. > It was already quite tricky to get the hybrid PE-COFF/bare metal image working, especially because the stub code is shared between x86, arm64 and ARM, all of which have their own quirks when it comes to the early environment this code executes in. ^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH] ARM: Fix zImage file size not aligned with CONFIG_EFI_STUB enabled 2017-10-24 8:09 ` Ard Biesheuvel @ 2017-10-24 9:16 ` jeffy -1 siblings, 0 replies; 42+ messages in thread From: jeffy @ 2017-10-24 9:16 UTC (permalink / raw) To: linux-arm-kernel Hi guys, On 10/24/2017 04:09 PM, Ard Biesheuvel wrote: > The following patch appears to fix the issue as well: > > diff --git a/arch/arm/boot/compressed/vmlinux.lds.S > b/arch/arm/boot/compressed/vmlinux.lds.S > index 7a4c59154361..0e0f504e256e 100644 > --- a/arch/arm/boot/compressed/vmlinux.lds.S > +++ b/arch/arm/boot/compressed/vmlinux.lds.S > @@ -83,7 +83,9 @@ SECTIONS > __pecoff_data_rawsize = . - ADDR(.data); > #endif > > - _edata = .; > + .edata (NOLOAD) : { > + _edata = .; > + } > > _magic_sig = ZIMAGE_MAGIC(0x016f2818); > _magic_start = ZIMAGE_MAGIC(_start); > > because the NOLOAD triggers the linker to emit all PROGBITS sections > before _edata, including unknown ones. > > E.g., in my binary, it gives me > > 00798020 D __pecoff_data_end > 00798200 d __ksymtab_sort > 00798208 B __bss_start > 00798208 D _edata this works too: +++ b/arch/arm/boot/compressed/vmlinux.lds.S @@ -70,10 +70,6 @@ SECTIONS .got : { *(.got) } _got_end = .; - /* ensure the zImage file size is always a multiple of 64 bits */ - /* (without a dummy byte, ld just ignores the empty section) */ - .pad : { BYTE(0); . = ALIGN(8); } - #ifdef CONFIG_EFI_STUB .data : ALIGN(4096) { __pecoff_data_start = .; @@ -93,7 +89,10 @@ SECTIONS __pecoff_data_rawsize = . - ADDR(.data); #endif - _edata = .; + /* ensure the zImage file size is always a multiple of 64 bits */ + .edata : ALIGN(8) { + _edata = .; + } 339: 006cf200 8 OBJECT LOCAL DEFAULT 9 __ksymtab_sort ... 528: 006cf208 0 NOTYPE GLOBAL DEFAULT 9 _edata ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH] ARM: Fix zImage file size not aligned with CONFIG_EFI_STUB enabled @ 2017-10-24 9:16 ` jeffy 0 siblings, 0 replies; 42+ messages in thread From: jeffy @ 2017-10-24 9:16 UTC (permalink / raw) To: Ard Biesheuvel, Russell King - ARM Linux Cc: Ingo Molnar, Chris Zhong, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org Hi guys, On 10/24/2017 04:09 PM, Ard Biesheuvel wrote: > The following patch appears to fix the issue as well: > > diff --git a/arch/arm/boot/compressed/vmlinux.lds.S > b/arch/arm/boot/compressed/vmlinux.lds.S > index 7a4c59154361..0e0f504e256e 100644 > --- a/arch/arm/boot/compressed/vmlinux.lds.S > +++ b/arch/arm/boot/compressed/vmlinux.lds.S > @@ -83,7 +83,9 @@ SECTIONS > __pecoff_data_rawsize = . - ADDR(.data); > #endif > > - _edata = .; > + .edata (NOLOAD) : { > + _edata = .; > + } > > _magic_sig = ZIMAGE_MAGIC(0x016f2818); > _magic_start = ZIMAGE_MAGIC(_start); > > because the NOLOAD triggers the linker to emit all PROGBITS sections > before _edata, including unknown ones. > > E.g., in my binary, it gives me > > 00798020 D __pecoff_data_end > 00798200 d __ksymtab_sort > 00798208 B __bss_start > 00798208 D _edata this works too: +++ b/arch/arm/boot/compressed/vmlinux.lds.S @@ -70,10 +70,6 @@ SECTIONS .got : { *(.got) } _got_end = .; - /* ensure the zImage file size is always a multiple of 64 bits */ - /* (without a dummy byte, ld just ignores the empty section) */ - .pad : { BYTE(0); . = ALIGN(8); } - #ifdef CONFIG_EFI_STUB .data : ALIGN(4096) { __pecoff_data_start = .; @@ -93,7 +89,10 @@ SECTIONS __pecoff_data_rawsize = . - ADDR(.data); #endif - _edata = .; + /* ensure the zImage file size is always a multiple of 64 bits */ + .edata : ALIGN(8) { + _edata = .; + } 339: 006cf200 8 OBJECT LOCAL DEFAULT 9 __ksymtab_sort ... 528: 006cf208 0 NOTYPE GLOBAL DEFAULT 9 _edata ^ permalink raw reply [flat|nested] 42+ messages in thread
end of thread, other threads:[~2017-10-24 10:03 UTC | newest] Thread overview: 42+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-10-18 5:01 [PATCH] ARM: Fix zImage file size not aligned with CONFIG_EFI_STUB enabled Jeffy Chen 2017-10-18 5:01 ` Jeffy Chen 2017-10-18 6:19 ` Chris Zhong 2017-10-18 6:19 ` Chris Zhong 2017-10-22 11:01 ` Ard Biesheuvel 2017-10-22 11:01 ` Ard Biesheuvel 2017-10-22 12:47 ` Russell King - ARM Linux 2017-10-22 12:47 ` Russell King - ARM Linux 2017-10-22 13:01 ` Ard Biesheuvel 2017-10-22 13:01 ` Ard Biesheuvel 2017-10-23 3:26 ` jeffy 2017-10-23 3:26 ` jeffy 2017-10-23 8:50 ` Russell King - ARM Linux 2017-10-23 8:50 ` Russell King - ARM Linux 2017-10-23 10:24 ` jeffy 2017-10-23 10:24 ` jeffy 2017-10-23 10:50 ` Russell King - ARM Linux 2017-10-23 10:50 ` Russell King - ARM Linux 2017-10-23 11:45 ` Russell King - ARM Linux 2017-10-23 11:45 ` Russell King - ARM Linux 2017-10-24 8:09 ` Ard Biesheuvel 2017-10-24 8:09 ` Ard Biesheuvel 2017-10-24 9:09 ` Russell King - ARM Linux 2017-10-24 9:09 ` Russell King - ARM Linux 2017-10-24 9:13 ` Ard Biesheuvel 2017-10-24 9:13 ` Ard Biesheuvel 2017-10-24 9:22 ` Russell King - ARM Linux 2017-10-24 9:22 ` Russell King - ARM Linux 2017-10-24 9:26 ` Ard Biesheuvel 2017-10-24 9:26 ` Ard Biesheuvel 2017-10-24 9:30 ` Ard Biesheuvel 2017-10-24 9:30 ` Ard Biesheuvel 2017-10-24 9:38 ` Russell King - ARM Linux 2017-10-24 9:38 ` Russell King - ARM Linux 2017-10-24 9:44 ` Ard Biesheuvel 2017-10-24 9:44 ` Ard Biesheuvel 2017-10-24 9:54 ` Russell King - ARM Linux 2017-10-24 9:54 ` Russell King - ARM Linux 2017-10-24 10:03 ` Ard Biesheuvel 2017-10-24 10:03 ` Ard Biesheuvel 2017-10-24 9:16 ` jeffy 2017-10-24 9:16 ` jeffy
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.