linux-efi.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] efi: Include a .bss section within the PE/COFF headers
@ 2014-07-09 21:41 Michael Brown
       [not found] ` <1404942094-29447-1-git-send-email-mbrown-OViyBiuKJBuK421+ScFKDQ@public.gmane.org>
  0 siblings, 1 reply; 9+ messages in thread
From: Michael Brown @ 2014-07-09 21:41 UTC (permalink / raw)
  To: linux-efi-u79uwXL29TY76Z2rM5mHXA; +Cc: Michael Brown

The PE/COFF headers currently describe only the initialised-data
portions of the image, and result in no space being allocated for the
uninitialised-data portions.  Consequently, the EFI boot stub will end
up overwriting unexpected areas of memory, with unpredictable results.

Fix by including a .bss section in the PE/COFF headers (functionally
equivalent to the init_size field in the bzImage header).

Signed-off-by: Michael Brown <mbrown-OViyBiuKJBuK421+ScFKDQ@public.gmane.org>
---
 arch/x86/boot/header.S      | 26 ++++++++++++++++++++++----
 arch/x86/boot/tools/build.c | 37 +++++++++++++++++++++++++++++--------
 2 files changed, 51 insertions(+), 12 deletions(-)

diff --git a/arch/x86/boot/header.S b/arch/x86/boot/header.S
index 84c2234..7a6d43a 100644
--- a/arch/x86/boot/header.S
+++ b/arch/x86/boot/header.S
@@ -91,10 +91,9 @@ bs_die:
 
 	.section ".bsdata", "a"
 bugger_off_msg:
-	.ascii	"Direct floppy boot is not supported. "
-	.ascii	"Use a boot loader program instead.\r\n"
+	.ascii	"Use a boot loader.\r\n"
 	.ascii	"\n"
-	.ascii	"Remove disk and press any key to reboot ...\r\n"
+	.ascii	"Remove disk and press any key to reboot...\r\n"
 	.byte	0
 
 #ifdef CONFIG_EFI_STUB
@@ -108,7 +107,7 @@ coff_header:
 #else
 	.word	0x8664				# x86-64
 #endif
-	.word	3				# nr_sections
+	.word	4				# nr_sections
 	.long	0 				# TimeDateStamp
 	.long	0				# PointerToSymbolTable
 	.long	1				# NumberOfSymbols
@@ -250,6 +249,25 @@ section_table:
 	.word	0				# NumberOfLineNumbers
 	.long	0x60500020			# Characteristics (section flags)
 
+	#
+	# The offset & size fields are filled in by build.c.
+	#
+	.ascii	".bss"
+	.byte	0
+	.byte	0
+	.byte	0
+	.byte	0
+	.long	0
+	.long	0x0
+	.long	0				# Size of initialized data
+						# on disk
+	.long	0x0
+	.long	0				# PointerToRelocations
+	.long	0				# PointerToLineNumbers
+	.word	0				# NumberOfRelocations
+	.word	0				# NumberOfLineNumbers
+	.long	0xc8000080			# Characteristics (section flags)
+
 #endif /* CONFIG_EFI_STUB */
 
 	# Kernel attributes; used by setup.  This is part 1 of the
diff --git a/arch/x86/boot/tools/build.c b/arch/x86/boot/tools/build.c
index 1a2f212..0afa9ac 100644
--- a/arch/x86/boot/tools/build.c
+++ b/arch/x86/boot/tools/build.c
@@ -143,7 +143,7 @@ static void usage(void)
 
 #ifdef CONFIG_EFI_STUB
 
-static void update_pecoff_section_header(char *section_name, u32 offset, u32 size)
+static void update_pecoff_section_header_fields(char *section_name, u32 vma, u32 size, u32 datasz, u32 offset)
 {
 	unsigned int pe_header;
 	unsigned short num_sections;
@@ -164,10 +164,10 @@ static void update_pecoff_section_header(char *section_name, u32 offset, u32 siz
 			put_unaligned_le32(size, section + 0x8);
 
 			/* section header vma field */
-			put_unaligned_le32(offset, section + 0xc);
+			put_unaligned_le32(vma, section + 0xc);
 
 			/* section header 'size of initialised data' field */
-			put_unaligned_le32(size, section + 0x10);
+			put_unaligned_le32(datasz, section + 0x10);
 
 			/* section header 'file offset' field */
 			put_unaligned_le32(offset, section + 0x14);
@@ -179,6 +179,11 @@ static void update_pecoff_section_header(char *section_name, u32 offset, u32 siz
 	}
 }
 
+static void update_pecoff_section_header(char *section_name, u32 offset, u32 size)
+{
+	update_pecoff_section_header_fields(section_name, offset, size, size, offset);
+}
+
 static void update_pecoff_setup_and_reloc(unsigned int size)
 {
 	u32 setup_offset = 0x200;
@@ -203,9 +208,6 @@ static void update_pecoff_text(unsigned int text_start, unsigned int file_sz)
 
 	pe_header = get_unaligned_le32(&buf[0x3c]);
 
-	/* Size of image */
-	put_unaligned_le32(file_sz, &buf[pe_header + 0x50]);
-
 	/*
 	 * Size of code: Subtract the size of the first sector (512 bytes)
 	 * which includes the header.
@@ -220,6 +222,22 @@ static void update_pecoff_text(unsigned int text_start, unsigned int file_sz)
 	update_pecoff_section_header(".text", text_start, text_sz);
 }
 
+static void update_pecoff_bss(unsigned int file_sz, unsigned int init_sz)
+{
+	unsigned int pe_header;
+	unsigned int bss_sz = init_sz - file_sz;
+
+	pe_header = get_unaligned_le32(&buf[0x3c]);
+
+	/* Size of uninitialized data */
+	put_unaligned_le32(bss_sz, &buf[pe_header + 0x24]);
+
+	/* Size of image */
+	put_unaligned_le32(init_sz, &buf[pe_header + 0x50]);
+
+	update_pecoff_section_header_fields(".bss", file_sz, bss_sz, 0, 0);
+}
+
 static int reserve_pecoff_reloc_section(int c)
 {
 	/* Reserve 0x20 bytes for .reloc section */
@@ -259,6 +277,7 @@ static void efi_stub_entry_update(void)
 static inline void update_pecoff_setup_and_reloc(unsigned int size) {}
 static inline void update_pecoff_text(unsigned int text_start,
 				      unsigned int file_sz) {}
+static inline update_pecoff_bss(unsigned int file_sz, unsigned int init_sz) {}
 static inline void efi_stub_defaults(void) {}
 static inline void efi_stub_entry_update(void) {}
 
@@ -310,7 +329,7 @@ static void parse_zoffset(char *fname)
 
 int main(int argc, char ** argv)
 {
-	unsigned int i, sz, setup_sectors;
+	unsigned int i, sz, setup_sectors, init_sz;
 	int c;
 	u32 sys_size;
 	struct stat sb;
@@ -376,7 +395,9 @@ int main(int argc, char ** argv)
 	buf[0x1f1] = setup_sectors-1;
 	put_unaligned_le32(sys_size, &buf[0x1f4]);
 
-	update_pecoff_text(setup_sectors * 512, sz + i + ((sys_size * 16) - sz));
+	update_pecoff_text(setup_sectors * 512, i + (sys_size * 16));
+	init_sz = get_unaligned_le32(&buf[0x260]);
+	update_pecoff_bss(i + (sys_size * 16), init_sz);
 
 	efi_stub_entry_update();
 
-- 
1.8.4.5

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH] efi: Include a .bss section within the PE/COFF headers
       [not found] ` <1404942094-29447-1-git-send-email-mbrown-OViyBiuKJBuK421+ScFKDQ@public.gmane.org>
@ 2014-07-09 22:20   ` Michael Brown
       [not found]     ` <53BDC038.1090003-OViyBiuKJBuK421+ScFKDQ@public.gmane.org>
  2014-07-10 10:34   ` Matt Fleming
  1 sibling, 1 reply; 9+ messages in thread
From: Michael Brown @ 2014-07-09 22:20 UTC (permalink / raw)
  To: Michael Brown, linux-efi-u79uwXL29TY76Z2rM5mHXA, H. Peter Anvin

On 09/07/14 22:41, Michael Brown wrote:
> The PE/COFF headers currently describe only the initialised-data
> portions of the image, and result in no space being allocated for the
> uninitialised-data portions.  Consequently, the EFI boot stub will end
> up overwriting unexpected areas of memory, with unpredictable results.
>
> Fix by including a .bss section in the PE/COFF headers (functionally
> equivalent to the init_size field in the bzImage header).

Following on from this: hpa mentioned via IRC that we should also take 
alignment into account.  I am unsure if init_size already includes 
padding for alignment; on my sample kernel init_size is >16MB (with 16MB 
alignment), so it looks plausible to me that alignment is already 
accounted for.

If not, then the following trivial patch exposes the desired alignment 
via the PE/COFF headers:

diff --git a/arch/x86/boot/header.S b/arch/x86/boot/header.S
index 7a6d43a..16ef025 100644
--- a/arch/x86/boot/header.S
+++ b/arch/x86/boot/header.S
@@ -154,7 +154,7 @@ extra_header_fields:
  #else
  	.quad	0				# ImageBase
  #endif
-	.long	0x20				# SectionAlignment
+	.long	CONFIG_PHYSICAL_ALIGN		# SectionAlignment
  	.long	0x20				# FileAlignment
  	.word	0				# MajorOperatingSystemVersion
  	.word	0				# MinorOperatingSystemVersion
-- 
1.8.4.5

Michael

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH] efi: Include a .bss section within the PE/COFF headers
       [not found]     ` <53BDC038.1090003-OViyBiuKJBuK421+ScFKDQ@public.gmane.org>
@ 2014-07-09 22:41       ` H. Peter Anvin
  0 siblings, 0 replies; 9+ messages in thread
From: H. Peter Anvin @ 2014-07-09 22:41 UTC (permalink / raw)
  To: Michael Brown, linux-efi-u79uwXL29TY76Z2rM5mHXA

init_size does not include any kind of alignment padding.

On July 9, 2014 3:20:40 PM PDT, Michael Brown <mbrown-OViyBiuKJBuK421+ScFKDQ@public.gmane.org> wrote:
>On 09/07/14 22:41, Michael Brown wrote:
>> The PE/COFF headers currently describe only the initialised-data
>> portions of the image, and result in no space being allocated for the
>> uninitialised-data portions.  Consequently, the EFI boot stub will
>end
>> up overwriting unexpected areas of memory, with unpredictable
>results.
>>
>> Fix by including a .bss section in the PE/COFF headers (functionally
>> equivalent to the init_size field in the bzImage header).
>
>Following on from this: hpa mentioned via IRC that we should also take 
>alignment into account.  I am unsure if init_size already includes 
>padding for alignment; on my sample kernel init_size is >16MB (with
>16MB 
>alignment), so it looks plausible to me that alignment is already 
>accounted for.
>
>If not, then the following trivial patch exposes the desired alignment 
>via the PE/COFF headers:
>
>diff --git a/arch/x86/boot/header.S b/arch/x86/boot/header.S
>index 7a6d43a..16ef025 100644
>--- a/arch/x86/boot/header.S
>+++ b/arch/x86/boot/header.S
>@@ -154,7 +154,7 @@ extra_header_fields:
>  #else
>  	.quad	0				# ImageBase
>  #endif
>-	.long	0x20				# SectionAlignment
>+	.long	CONFIG_PHYSICAL_ALIGN		# SectionAlignment
>  	.long	0x20				# FileAlignment
>  	.word	0				# MajorOperatingSystemVersion
>  	.word	0				# MinorOperatingSystemVersion

-- 
Sent from my mobile phone.  Please pardon brevity and lack of formatting.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] efi: Include a .bss section within the PE/COFF headers
       [not found] ` <1404942094-29447-1-git-send-email-mbrown-OViyBiuKJBuK421+ScFKDQ@public.gmane.org>
  2014-07-09 22:20   ` Michael Brown
@ 2014-07-10 10:34   ` Matt Fleming
       [not found]     ` <20140710103431.GE15932-HNK1S37rvNbeXh+fF434Mdi2O/JbrIOy@public.gmane.org>
  1 sibling, 1 reply; 9+ messages in thread
From: Matt Fleming @ 2014-07-10 10:34 UTC (permalink / raw)
  To: Michael Brown
  Cc: linux-efi-u79uwXL29TY76Z2rM5mHXA, H. Peter Anvin,
	Thomas Bächler

On Wed, 09 Jul, at 10:41:34PM, Michael Brown wrote:
> The PE/COFF headers currently describe only the initialised-data
> portions of the image, and result in no space being allocated for the
> uninitialised-data portions.  Consequently, the EFI boot stub will end
> up overwriting unexpected areas of memory, with unpredictable results.
> 
> Fix by including a .bss section in the PE/COFF headers (functionally
> equivalent to the init_size field in the bzImage header).
> 
> Signed-off-by: Michael Brown <mbrown-OViyBiuKJBuK421+ScFKDQ@public.gmane.org>
> ---
>  arch/x86/boot/header.S      | 26 ++++++++++++++++++++++----
>  arch/x86/boot/tools/build.c | 37 +++++++++++++++++++++++++++++--------
>  2 files changed, 51 insertions(+), 12 deletions(-)

Yeah ouch, that's a particularly bad bug. Thanks Michael, this fix looks
great.

I've placed this in the urgent EFI queue and tagged it for stable.

Thomas, the patch in question is here this one,

  http://git.kernel.org/cgit/linux/kernel/git/mfleming/efi.git/commit/?h=urgent&id=db0f1ff0ee1750cc52ead0ba1ddf95c47b3bd133

it would be good if you could carry it in archlinux for some additional
testing. I suspect this fix may solve some of the problems people have
reported in,

  https://bugzilla.kernel.org/show_bug.cgi?id=68761

Let me know if you'd like a backported version.

-- 
Matt Fleming, Intel Open Source Technology Center

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] efi: Include a .bss section within the PE/COFF headers
       [not found]     ` <20140710103431.GE15932-HNK1S37rvNbeXh+fF434Mdi2O/JbrIOy@public.gmane.org>
@ 2014-07-10 14:48       ` Matt Fleming
       [not found]         ` <20140710144815.GG15932-HNK1S37rvNbeXh+fF434Mdi2O/JbrIOy@public.gmane.org>
  0 siblings, 1 reply; 9+ messages in thread
From: Matt Fleming @ 2014-07-10 14:48 UTC (permalink / raw)
  To: Michael Brown
  Cc: linux-efi-u79uwXL29TY76Z2rM5mHXA, H. Peter Anvin,
	Thomas Bächler

On Thu, 10 Jul, at 11:34:31AM, Matt Fleming wrote:
> 
> Thomas, the patch in question is here this one,
> 
>   http://git.kernel.org/cgit/linux/kernel/git/mfleming/efi.git/commit/?h=urgent&id=db0f1ff0ee1750cc52ead0ba1ddf95c47b3bd133

Correction, it's now this one,

  http://git.kernel.org/cgit/linux/kernel/git/mfleming/efi.git/commit/?h=urgent&id=c7fb93ec51d462ec3540a729ba446663c26a0505

-- 
Matt Fleming, Intel Open Source Technology Center

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] efi: Include a .bss section within the PE/COFF headers
       [not found]         ` <20140710144815.GG15932-HNK1S37rvNbeXh+fF434Mdi2O/JbrIOy@public.gmane.org>
@ 2014-07-11 17:22           ` Thomas Bächler
  0 siblings, 0 replies; 9+ messages in thread
From: Thomas Bächler @ 2014-07-11 17:22 UTC (permalink / raw)
  To: Matt Fleming, Michael Brown
  Cc: linux-efi-u79uwXL29TY76Z2rM5mHXA, H. Peter Anvin,
	Thomas Bächler

[-- Attachment #1: Type: text/plain, Size: 918 bytes --]

Am 10.07.2014 16:48, schrieb Matt Fleming:
> On Thu, 10 Jul, at 11:34:31AM, Matt Fleming wrote:
>>
>> Thomas, the patch in question is here this one,
>>
>>   http://git.kernel.org/cgit/linux/kernel/git/mfleming/efi.git/commit/?h=urgent&id=db0f1ff0ee1750cc52ead0ba1ddf95c47b3bd133
> 
> Correction, it's now this one,
> 
>   http://git.kernel.org/cgit/linux/kernel/git/mfleming/efi.git/commit/?h=urgent&id=c7fb93ec51d462ec3540a729ba446663c26a0505
> 

Hello Matt, thanks for forwarding the patch. We applied it to our linux
3.15 package, and at least one user still reported failure. Due to the
random nature of the problem, we will need to wait for a few builds to
see if the problem occurs less often.

See here https://bugs.archlinux.org/task/33745#comment125251:

"Comment by Steven V (steabert) - Friday, 11 July 2014, 15:59 GMT+2
For me, 3.15.5-2 still doesn't work (Dell Latitude E4310)."



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 884 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH] efi: Include a .bss section within the PE/COFF headers
@ 2014-07-28 13:21 Michael Brown
       [not found] ` <1406553713-7479-1-git-send-email-mbrown-OViyBiuKJBuK421+ScFKDQ@public.gmane.org>
  0 siblings, 1 reply; 9+ messages in thread
From: Michael Brown @ 2014-07-28 13:21 UTC (permalink / raw)
  To: stable-u79uwXL29TY76Z2rM5mHXA
  Cc: Michael Brown, linux-efi-u79uwXL29TY76Z2rM5mHXA

commit c7fb93ec51d462ec3540a729ba446663c26a0505 upstream

The PE/COFF headers currently describe only the initialised-data
portions of the image, and result in no space being allocated for the
uninitialised-data portions.  Consequently, the EFI boot stub will end
up overwriting unexpected areas of memory, with unpredictable results.

Fix by including a .bss section in the PE/COFF headers (functionally
equivalent to the init_size field in the bzImage header).

Signed-off-by: Michael Brown <mbrown-OViyBiuKJBuK421+ScFKDQ@public.gmane.org>
Cc: linux-efi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Cc: stable-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
---
 arch/x86/boot/header.S      | 26 ++++++++++++++++++++++----
 arch/x86/boot/tools/build.c | 37 ++++++++++++++++++++++++++++++-------
 2 files changed, 52 insertions(+), 11 deletions(-)

diff --git a/arch/x86/boot/header.S b/arch/x86/boot/header.S
index ec3b8ba..04da6c2 100644
--- a/arch/x86/boot/header.S
+++ b/arch/x86/boot/header.S
@@ -91,10 +91,9 @@ bs_die:
 
 	.section ".bsdata", "a"
 bugger_off_msg:
-	.ascii	"Direct floppy boot is not supported. "
-	.ascii	"Use a boot loader program instead.\r\n"
+	.ascii	"Use a boot loader.\r\n"
 	.ascii	"\n"
-	.ascii	"Remove disk and press any key to reboot ...\r\n"
+	.ascii	"Remove disk and press any key to reboot...\r\n"
 	.byte	0
 
 #ifdef CONFIG_EFI_STUB
@@ -108,7 +107,7 @@ coff_header:
 #else
 	.word	0x8664				# x86-64
 #endif
-	.word	3				# nr_sections
+	.word	4				# nr_sections
 	.long	0 				# TimeDateStamp
 	.long	0				# PointerToSymbolTable
 	.long	1				# NumberOfSymbols
@@ -250,6 +249,25 @@ section_table:
 	.word	0				# NumberOfLineNumbers
 	.long	0x60500020			# Characteristics (section flags)
 
+	#
+	# The offset & size fields are filled in by build.c.
+	#
+	.ascii	".bss"
+	.byte	0
+	.byte	0
+	.byte	0
+	.byte	0
+	.long	0
+	.long	0x0
+	.long	0				# Size of initialized data
+						# on disk
+	.long	0x0
+	.long	0				# PointerToRelocations
+	.long	0				# PointerToLineNumbers
+	.word	0				# NumberOfRelocations
+	.word	0				# NumberOfLineNumbers
+	.long	0xc8000080			# Characteristics (section flags)
+
 #endif /* CONFIG_EFI_STUB */
 
 	# Kernel attributes; used by setup.  This is part 1 of the
diff --git a/arch/x86/boot/tools/build.c b/arch/x86/boot/tools/build.c
index 8e15b22..3dafaeb 100644
--- a/arch/x86/boot/tools/build.c
+++ b/arch/x86/boot/tools/build.c
@@ -142,7 +142,7 @@ static void usage(void)
 
 #ifdef CONFIG_EFI_STUB
 
-static void update_pecoff_section_header(char *section_name, u32 offset, u32 size)
+static void update_pecoff_section_header_fields(char *section_name, u32 vma, u32 size, u32 datasz, u32 offset)
 {
 	unsigned int pe_header;
 	unsigned short num_sections;
@@ -163,10 +163,10 @@ static void update_pecoff_section_header(char *section_name, u32 offset, u32 siz
 			put_unaligned_le32(size, section + 0x8);
 
 			/* section header vma field */
-			put_unaligned_le32(offset, section + 0xc);
+			put_unaligned_le32(vma, section + 0xc);
 
 			/* section header 'size of initialised data' field */
-			put_unaligned_le32(size, section + 0x10);
+			put_unaligned_le32(datasz, section + 0x10);
 
 			/* section header 'file offset' field */
 			put_unaligned_le32(offset, section + 0x14);
@@ -178,6 +178,11 @@ static void update_pecoff_section_header(char *section_name, u32 offset, u32 siz
 	}
 }
 
+static void update_pecoff_section_header(char *section_name, u32 offset, u32 size)
+{
+	update_pecoff_section_header_fields(section_name, offset, size, size, offset);
+}
+
 static void update_pecoff_setup_and_reloc(unsigned int size)
 {
 	u32 setup_offset = 0x200;
@@ -202,9 +207,6 @@ static void update_pecoff_text(unsigned int text_start, unsigned int file_sz)
 
 	pe_header = get_unaligned_le32(&buf[0x3c]);
 
-	/* Size of image */
-	put_unaligned_le32(file_sz, &buf[pe_header + 0x50]);
-
 	/*
 	 * Size of code: Subtract the size of the first sector (512 bytes)
 	 * which includes the header.
@@ -219,6 +221,22 @@ static void update_pecoff_text(unsigned int text_start, unsigned int file_sz)
 	update_pecoff_section_header(".text", text_start, text_sz);
 }
 
+static void update_pecoff_bss(unsigned int file_sz, unsigned int init_sz)
+{
+	unsigned int pe_header;
+	unsigned int bss_sz = init_sz - file_sz;
+
+	pe_header = get_unaligned_le32(&buf[0x3c]);
+
+	/* Size of uninitialized data */
+	put_unaligned_le32(bss_sz, &buf[pe_header + 0x24]);
+
+	/* Size of image */
+	put_unaligned_le32(init_sz, &buf[pe_header + 0x50]);
+
+	update_pecoff_section_header_fields(".bss", file_sz, bss_sz, 0, 0);
+}
+
 #endif /* CONFIG_EFI_STUB */
 
 
@@ -270,6 +288,9 @@ int main(int argc, char ** argv)
 	int fd;
 	void *kernel;
 	u32 crc = 0xffffffffUL;
+#ifdef CONFIG_EFI_STUB
+	unsigned int init_sz;
+#endif
 
 	/* Defaults for old kernel */
 #ifdef CONFIG_X86_32
@@ -343,7 +364,9 @@ int main(int argc, char ** argv)
 	put_unaligned_le32(sys_size, &buf[0x1f4]);
 
 #ifdef CONFIG_EFI_STUB
-	update_pecoff_text(setup_sectors * 512, sz + i + ((sys_size * 16) - sz));
+	update_pecoff_text(setup_sectors * 512, i + (sys_size * 16));
+	init_sz = get_unaligned_le32(&buf[0x260]);
+	update_pecoff_bss(i + (sys_size * 16), init_sz);
 
 #ifdef CONFIG_X86_64 /* Yes, this is really how we defined it :( */
 	efi_stub_entry -= 0x200;
-- 
1.8.4.5

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH] efi: Include a .bss section within the PE/COFF headers
       [not found] ` <1406553713-7479-1-git-send-email-mbrown-OViyBiuKJBuK421+ScFKDQ@public.gmane.org>
@ 2014-07-28 22:54   ` H. Peter Anvin
  2014-07-30 13:57   ` Luis Henriques
  1 sibling, 0 replies; 9+ messages in thread
From: H. Peter Anvin @ 2014-07-28 22:54 UTC (permalink / raw)
  To: Michael Brown, Matt Fleming
  Cc: linux-efi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org

On 07/28/2014 06:21 AM, Michael Brown wrote:
> commit c7fb93ec51d462ec3540a729ba446663c26a0505 upstream
> 
> The PE/COFF headers currently describe only the initialised-data
> portions of the image, and result in no space being allocated for the
> uninitialised-data portions.  Consequently, the EFI boot stub will end
> up overwriting unexpected areas of memory, with unpredictable results.
> 
> Fix by including a .bss section in the PE/COFF headers (functionally
> equivalent to the init_size field in the bzImage header).
> 
> Signed-off-by: Michael Brown <mbrown-OViyBiuKJBuK421+ScFKDQ@public.gmane.org>
> Cc: linux-efi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Cc: stable-u79uwXL29TY76Z2rM5mHXA@public.gmane.org

Just FYI: please enclose the email addresses in angle brackets even if
there is no plain name.  Otherwise you have a tendency to break various
people's scripts in unpredictable ways.

	-hpa

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] efi: Include a .bss section within the PE/COFF headers
       [not found] ` <1406553713-7479-1-git-send-email-mbrown-OViyBiuKJBuK421+ScFKDQ@public.gmane.org>
  2014-07-28 22:54   ` H. Peter Anvin
@ 2014-07-30 13:57   ` Luis Henriques
  1 sibling, 0 replies; 9+ messages in thread
From: Luis Henriques @ 2014-07-30 13:57 UTC (permalink / raw)
  To: Michael Brown
  Cc: stable-u79uwXL29TY76Z2rM5mHXA, linux-efi-u79uwXL29TY76Z2rM5mHXA

On Mon, Jul 28, 2014 at 02:21:53PM +0100, Michael Brown wrote:
> commit c7fb93ec51d462ec3540a729ba446663c26a0505 upstream
>

Thanks, I'll use this backport for the 3.11 kernel as well.

Cheers,
--
Luís

> The PE/COFF headers currently describe only the initialised-data
> portions of the image, and result in no space being allocated for the
> uninitialised-data portions.  Consequently, the EFI boot stub will end
> up overwriting unexpected areas of memory, with unpredictable results.
> 
> Fix by including a .bss section in the PE/COFF headers (functionally
> equivalent to the init_size field in the bzImage header).
> 
> Signed-off-by: Michael Brown <mbrown-OViyBiuKJBuK421+ScFKDQ@public.gmane.org>
> Cc: linux-efi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Cc: stable-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> ---
>  arch/x86/boot/header.S      | 26 ++++++++++++++++++++++----
>  arch/x86/boot/tools/build.c | 37 ++++++++++++++++++++++++++++++-------
>  2 files changed, 52 insertions(+), 11 deletions(-)
> 
> diff --git a/arch/x86/boot/header.S b/arch/x86/boot/header.S
> index ec3b8ba..04da6c2 100644
> --- a/arch/x86/boot/header.S
> +++ b/arch/x86/boot/header.S
> @@ -91,10 +91,9 @@ bs_die:
>  
>  	.section ".bsdata", "a"
>  bugger_off_msg:
> -	.ascii	"Direct floppy boot is not supported. "
> -	.ascii	"Use a boot loader program instead.\r\n"
> +	.ascii	"Use a boot loader.\r\n"
>  	.ascii	"\n"
> -	.ascii	"Remove disk and press any key to reboot ...\r\n"
> +	.ascii	"Remove disk and press any key to reboot...\r\n"
>  	.byte	0
>  
>  #ifdef CONFIG_EFI_STUB
> @@ -108,7 +107,7 @@ coff_header:
>  #else
>  	.word	0x8664				# x86-64
>  #endif
> -	.word	3				# nr_sections
> +	.word	4				# nr_sections
>  	.long	0 				# TimeDateStamp
>  	.long	0				# PointerToSymbolTable
>  	.long	1				# NumberOfSymbols
> @@ -250,6 +249,25 @@ section_table:
>  	.word	0				# NumberOfLineNumbers
>  	.long	0x60500020			# Characteristics (section flags)
>  
> +	#
> +	# The offset & size fields are filled in by build.c.
> +	#
> +	.ascii	".bss"
> +	.byte	0
> +	.byte	0
> +	.byte	0
> +	.byte	0
> +	.long	0
> +	.long	0x0
> +	.long	0				# Size of initialized data
> +						# on disk
> +	.long	0x0
> +	.long	0				# PointerToRelocations
> +	.long	0				# PointerToLineNumbers
> +	.word	0				# NumberOfRelocations
> +	.word	0				# NumberOfLineNumbers
> +	.long	0xc8000080			# Characteristics (section flags)
> +
>  #endif /* CONFIG_EFI_STUB */
>  
>  	# Kernel attributes; used by setup.  This is part 1 of the
> diff --git a/arch/x86/boot/tools/build.c b/arch/x86/boot/tools/build.c
> index 8e15b22..3dafaeb 100644
> --- a/arch/x86/boot/tools/build.c
> +++ b/arch/x86/boot/tools/build.c
> @@ -142,7 +142,7 @@ static void usage(void)
>  
>  #ifdef CONFIG_EFI_STUB
>  
> -static void update_pecoff_section_header(char *section_name, u32 offset, u32 size)
> +static void update_pecoff_section_header_fields(char *section_name, u32 vma, u32 size, u32 datasz, u32 offset)
>  {
>  	unsigned int pe_header;
>  	unsigned short num_sections;
> @@ -163,10 +163,10 @@ static void update_pecoff_section_header(char *section_name, u32 offset, u32 siz
>  			put_unaligned_le32(size, section + 0x8);
>  
>  			/* section header vma field */
> -			put_unaligned_le32(offset, section + 0xc);
> +			put_unaligned_le32(vma, section + 0xc);
>  
>  			/* section header 'size of initialised data' field */
> -			put_unaligned_le32(size, section + 0x10);
> +			put_unaligned_le32(datasz, section + 0x10);
>  
>  			/* section header 'file offset' field */
>  			put_unaligned_le32(offset, section + 0x14);
> @@ -178,6 +178,11 @@ static void update_pecoff_section_header(char *section_name, u32 offset, u32 siz
>  	}
>  }
>  
> +static void update_pecoff_section_header(char *section_name, u32 offset, u32 size)
> +{
> +	update_pecoff_section_header_fields(section_name, offset, size, size, offset);
> +}
> +
>  static void update_pecoff_setup_and_reloc(unsigned int size)
>  {
>  	u32 setup_offset = 0x200;
> @@ -202,9 +207,6 @@ static void update_pecoff_text(unsigned int text_start, unsigned int file_sz)
>  
>  	pe_header = get_unaligned_le32(&buf[0x3c]);
>  
> -	/* Size of image */
> -	put_unaligned_le32(file_sz, &buf[pe_header + 0x50]);
> -
>  	/*
>  	 * Size of code: Subtract the size of the first sector (512 bytes)
>  	 * which includes the header.
> @@ -219,6 +221,22 @@ static void update_pecoff_text(unsigned int text_start, unsigned int file_sz)
>  	update_pecoff_section_header(".text", text_start, text_sz);
>  }
>  
> +static void update_pecoff_bss(unsigned int file_sz, unsigned int init_sz)
> +{
> +	unsigned int pe_header;
> +	unsigned int bss_sz = init_sz - file_sz;
> +
> +	pe_header = get_unaligned_le32(&buf[0x3c]);
> +
> +	/* Size of uninitialized data */
> +	put_unaligned_le32(bss_sz, &buf[pe_header + 0x24]);
> +
> +	/* Size of image */
> +	put_unaligned_le32(init_sz, &buf[pe_header + 0x50]);
> +
> +	update_pecoff_section_header_fields(".bss", file_sz, bss_sz, 0, 0);
> +}
> +
>  #endif /* CONFIG_EFI_STUB */
>  
>  
> @@ -270,6 +288,9 @@ int main(int argc, char ** argv)
>  	int fd;
>  	void *kernel;
>  	u32 crc = 0xffffffffUL;
> +#ifdef CONFIG_EFI_STUB
> +	unsigned int init_sz;
> +#endif
>  
>  	/* Defaults for old kernel */
>  #ifdef CONFIG_X86_32
> @@ -343,7 +364,9 @@ int main(int argc, char ** argv)
>  	put_unaligned_le32(sys_size, &buf[0x1f4]);
>  
>  #ifdef CONFIG_EFI_STUB
> -	update_pecoff_text(setup_sectors * 512, sz + i + ((sys_size * 16) - sz));
> +	update_pecoff_text(setup_sectors * 512, i + (sys_size * 16));
> +	init_sz = get_unaligned_le32(&buf[0x260]);
> +	update_pecoff_bss(i + (sys_size * 16), init_sz);
>  
>  #ifdef CONFIG_X86_64 /* Yes, this is really how we defined it :( */
>  	efi_stub_entry -= 0x200;
> -- 
> 1.8.4.5
> 
> --
> To unsubscribe from this list: send the line "unsubscribe stable" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2014-07-30 13:57 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-07-28 13:21 [PATCH] efi: Include a .bss section within the PE/COFF headers Michael Brown
     [not found] ` <1406553713-7479-1-git-send-email-mbrown-OViyBiuKJBuK421+ScFKDQ@public.gmane.org>
2014-07-28 22:54   ` H. Peter Anvin
2014-07-30 13:57   ` Luis Henriques
  -- strict thread matches above, loose matches on Subject: below --
2014-07-09 21:41 Michael Brown
     [not found] ` <1404942094-29447-1-git-send-email-mbrown-OViyBiuKJBuK421+ScFKDQ@public.gmane.org>
2014-07-09 22:20   ` Michael Brown
     [not found]     ` <53BDC038.1090003-OViyBiuKJBuK421+ScFKDQ@public.gmane.org>
2014-07-09 22:41       ` H. Peter Anvin
2014-07-10 10:34   ` Matt Fleming
     [not found]     ` <20140710103431.GE15932-HNK1S37rvNbeXh+fF434Mdi2O/JbrIOy@public.gmane.org>
2014-07-10 14:48       ` Matt Fleming
     [not found]         ` <20140710144815.GG15932-HNK1S37rvNbeXh+fF434Mdi2O/JbrIOy@public.gmane.org>
2014-07-11 17:22           ` Thomas Bächler

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).