[PATCH v2 0/1] x86/boot: save fields explicitly, zero out everything else

[PATCH v2 0/1] x86/boot: save fields explicitly, zero out everything else

[john.hubbard]

From: John Hubbard <jhubbard@nvidia.com>

Hi,

This uses the "save each field explicitly" approach that we discussed
during the first review [1]. As in [1], this is motivated by a desire
to clear the compiler warnings when building with gcc 9.

This is difficult to properly test. I've done a basic boot test, but
if there are actually errors in which items get zeroed or not, I don't
have a good test to uncover that.

[1] https://lore.kernel.org/r/alpine.DEB.2.21.1907260036500.1791@nanos.tec.linutronix.de

John Hubbard (1):
  x86/boot: save fields explicitly, zero out everything else

 arch/x86/include/asm/bootparam_utils.h | 62 +++++++++++++++++++-------
 1 file changed, 47 insertions(+), 15 deletions(-)

-- 
2.22.0

[PATCH v2] x86/boot: save fields explicitly, zero out everything else

[john.hubbard]

From: John Hubbard <jhubbard@nvidia.com>

Recent gcc compilers (gcc 9.1) generate warnings about an
out of bounds memset, if you trying memset across several fields
of a struct. This generated a couple of warnings on x86_64 builds.

Fix this by explicitly saving the fields in struct boot_params
that are intended to be preserved, and zeroing all the rest.

Suggested-by: Thomas Gleixner <tglx@linutronix.de>
Suggested-by: H. Peter Anvin <hpa@zytor.com>
Signed-off-by: John Hubbard <jhubbard@nvidia.com>
---
 arch/x86/include/asm/bootparam_utils.h | 62 +++++++++++++++++++-------
 1 file changed, 47 insertions(+), 15 deletions(-)

diff --git a/arch/x86/include/asm/bootparam_utils.h b/arch/x86/include/asm/bootparam_utils.h
index 101eb944f13c..514aee24b8de 100644
--- a/arch/x86/include/asm/bootparam_utils.h
+++ b/arch/x86/include/asm/bootparam_utils.h
@@ -18,6 +18,20 @@
  * Note: efi_info is commonly left uninitialized, but that field has a
  * private magic, so it is better to leave it unchanged.
  */
+
+#define sizeof_mbr(type, member) ({ sizeof(((type *)0)->member); })
+
+#define BOOT_PARAM_PRESERVE(struct_member)				\
+	{								\
+		.start = offsetof(struct boot_params, struct_member),	\
+		.len   = sizeof_mbr(struct boot_params, struct_member),	\
+	}
+
+struct boot_params_to_save {
+	unsigned int start;
+	unsigned int len;
+};
+
 static void sanitize_boot_params(struct boot_params *boot_params)
 {
 	/* 
@@ -35,21 +49,39 @@ static void sanitize_boot_params(struct boot_params *boot_params)
 	 * problems again.
 	 */
 	if (boot_params->sentinel) {
-		/* fields in boot_params are left uninitialized, clear them */
-		boot_params->acpi_rsdp_addr = 0;
-		memset(&boot_params->ext_ramdisk_image, 0,
-		       (char *)&boot_params->efi_info -
-			(char *)&boot_params->ext_ramdisk_image);
-		memset(&boot_params->kbd_status, 0,
-		       (char *)&boot_params->hdr -
-		       (char *)&boot_params->kbd_status);
-		memset(&boot_params->_pad7[0], 0,
-		       (char *)&boot_params->edd_mbr_sig_buffer[0] -
-			(char *)&boot_params->_pad7[0]);
-		memset(&boot_params->_pad8[0], 0,
-		       (char *)&boot_params->eddbuf[0] -
-			(char *)&boot_params->_pad8[0]);
-		memset(&boot_params->_pad9[0], 0, sizeof(boot_params->_pad9));
+		static struct boot_params scratch;
+		char *bp_base = (char *)boot_params;
+		char *save_base = (char *)&scratch;
+		int i;
+
+		const struct boot_params_to_save to_save[] = {
+			BOOT_PARAM_PRESERVE(screen_info),
+			BOOT_PARAM_PRESERVE(apm_bios_info),
+			BOOT_PARAM_PRESERVE(tboot_addr),
+			BOOT_PARAM_PRESERVE(ist_info),
+			BOOT_PARAM_PRESERVE(acpi_rsdp_addr),
+			BOOT_PARAM_PRESERVE(hd0_info),
+			BOOT_PARAM_PRESERVE(hd1_info),
+			BOOT_PARAM_PRESERVE(sys_desc_table),
+			BOOT_PARAM_PRESERVE(olpc_ofw_header),
+			BOOT_PARAM_PRESERVE(efi_info),
+			BOOT_PARAM_PRESERVE(alt_mem_k),
+			BOOT_PARAM_PRESERVE(scratch),
+			BOOT_PARAM_PRESERVE(e820_entries),
+			BOOT_PARAM_PRESERVE(eddbuf_entries),
+			BOOT_PARAM_PRESERVE(edd_mbr_sig_buf_entries),
+			BOOT_PARAM_PRESERVE(edd_mbr_sig_buffer),
+			BOOT_PARAM_PRESERVE(e820_table),
+			BOOT_PARAM_PRESERVE(eddbuf),
+		};
+
+		memset(&scratch, 0, sizeof(scratch));
+
+		for (i = 0; i < ARRAY_SIZE(to_save); i++)
+			memcpy(save_base + to_save[i].start,
+			       bp_base + to_save[i].start, to_save[i].len);
+
+		memcpy(boot_params, save_base, sizeof(*boot_params));
 	}
 }
 
-- 
2.22.0

Re: [PATCH v2 0/1] x86/boot: save fields explicitly, zero out everything else

[John Hubbard]

On 7/30/19 10:46 PM, john.hubbard@gmail.com wrote:
> From: John Hubbard <jhubbard@nvidia.com>
> 
> Hi,
> 
> This uses the "save each field explicitly" approach that we discussed
> during the first review [1]. As in [1], this is motivated by a desire
> to clear the compiler warnings when building with gcc 9.
> 
> This is difficult to properly test. I've done a basic boot test, but
> if there are actually errors in which items get zeroed or not, I don't
> have a good test to uncover that.


Also, if anyone has advice about any extra testing I could run on this,
please send it my way.

thanks,
-- 
John Hubbard
NVIDIA

> 
> [1] https://lore.kernel.org/r/alpine.DEB.2.21.1907260036500.1791@nanos.tec.linutronix.de
> 
> John Hubbard (1):
>   x86/boot: save fields explicitly, zero out everything else
> 
>  arch/x86/include/asm/bootparam_utils.h | 62 +++++++++++++++++++-------
>  1 file changed, 47 insertions(+), 15 deletions(-)
> 

RE: [PATCH v2] x86/boot: save fields explicitly, zero out everything else

[David Laight]

From: john.hubbard@gmail.com
> Sent: 31 July 2019 06:46
> 
> Recent gcc compilers (gcc 9.1) generate warnings about an
> out of bounds memset, if you trying memset across several fields
> of a struct. This generated a couple of warnings on x86_64 builds.
> 
> Fix this by explicitly saving the fields in struct boot_params
> that are intended to be preserved, and zeroing all the rest.
> 
> Suggested-by: Thomas Gleixner <tglx@linutronix.de>
> Suggested-by: H. Peter Anvin <hpa@zytor.com>
> Signed-off-by: John Hubbard <jhubbard@nvidia.com>
> ---
>  arch/x86/include/asm/bootparam_utils.h | 62 +++++++++++++++++++-------
>  1 file changed, 47 insertions(+), 15 deletions(-)
> 
> diff --git a/arch/x86/include/asm/bootparam_utils.h b/arch/x86/include/asm/bootparam_utils.h
> index 101eb944f13c..514aee24b8de 100644
> --- a/arch/x86/include/asm/bootparam_utils.h
> +++ b/arch/x86/include/asm/bootparam_utils.h
> @@ -18,6 +18,20 @@
>   * Note: efi_info is commonly left uninitialized, but that field has a
>   * private magic, so it is better to leave it unchanged.
>   */
> +
> +#define sizeof_mbr(type, member) ({ sizeof(((type *)0)->member); })
> +
> +#define BOOT_PARAM_PRESERVE(struct_member)				\
> +	{								\
> +		.start = offsetof(struct boot_params, struct_member),	\
> +		.len   = sizeof_mbr(struct boot_params, struct_member),	\
> +	}
> +
> +struct boot_params_to_save {
> +	unsigned int start;
> +	unsigned int len;
> +};
> +
>  static void sanitize_boot_params(struct boot_params *boot_params)
>  {
>  	/*
> @@ -35,21 +49,39 @@ static void sanitize_boot_params(struct boot_params *boot_params)
>  	 * problems again.
>  	 */
>  	if (boot_params->sentinel) {
> -		/* fields in boot_params are left uninitialized, clear them */
> -		boot_params->acpi_rsdp_addr = 0;
> -		memset(&boot_params->ext_ramdisk_image, 0,
> -		       (char *)&boot_params->efi_info -
> -			(char *)&boot_params->ext_ramdisk_image);
> -		memset(&boot_params->kbd_status, 0,
> -		       (char *)&boot_params->hdr -
> -		       (char *)&boot_params->kbd_status);
> -		memset(&boot_params->_pad7[0], 0,
> -		       (char *)&boot_params->edd_mbr_sig_buffer[0] -
> -			(char *)&boot_params->_pad7[0]);
> -		memset(&boot_params->_pad8[0], 0,
> -		       (char *)&boot_params->eddbuf[0] -
> -			(char *)&boot_params->_pad8[0]);
> -		memset(&boot_params->_pad9[0], 0, sizeof(boot_params->_pad9));
...

How about replacing the above first using:
#define zero_struct_fields(ptr, from, to) memset(&ptr->from, 0, (char *)&ptr->to - (char *)&ptr->from)
	zero_struct_fields(boot_params, ext_ramdisk_image, efi_info);
	...
Which is absolutely identical to the original code.

The replacing the define with:
	#define so(s, m) offsetof(typeof(*s), m)
	#define zero_struct_fields(ptr, from, to) memset((char *)ptr + so(ptr, from), 0, so(ptr, to) - so(ptr, from))
which gcc probably doesn't complain about, but should generate identical code again.
There might be an existing define for so().

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

[tip:x86/boot] x86/boot: Save fields explicitly, zero out everything else

[tip-bot for John Hubbard]

Commit-ID:  610666f0581557944c3abec93a7c125b8303442c
Gitweb:     https://git.kernel.org/tip/610666f0581557944c3abec93a7c125b8303442c
Author:     John Hubbard <jhubbard@nvidia.com>
AuthorDate: Tue, 30 Jul 2019 22:46:27 -0700
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Wed, 7 Aug 2019 15:16:04 +0200

x86/boot: Save fields explicitly, zero out everything else

Recent gcc compilers (gcc 9.1) generate warnings about an
out of bounds memset, if you trying memset across several fields
of a struct. This generated a couple of warnings on x86_64 builds.

Fix this by explicitly saving the fields in struct boot_params
that are intended to be preserved, and zeroing all the rest.

Suggested-by: Thomas Gleixner <tglx@linutronix.de>
Suggested-by: H. Peter Anvin <hpa@zytor.com>
Signed-off-by: John Hubbard <jhubbard@nvidia.com>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Link: https://lkml.kernel.org/r/20190731054627.5627-2-jhubbard@nvidia.com

---
 arch/x86/include/asm/bootparam_utils.h | 63 ++++++++++++++++++++++++++--------
 1 file changed, 48 insertions(+), 15 deletions(-)

diff --git a/arch/x86/include/asm/bootparam_utils.h b/arch/x86/include/asm/bootparam_utils.h
index 101eb944f13c..f5e90a849bca 100644
--- a/arch/x86/include/asm/bootparam_utils.h
+++ b/arch/x86/include/asm/bootparam_utils.h
@@ -18,6 +18,20 @@
  * Note: efi_info is commonly left uninitialized, but that field has a
  * private magic, so it is better to leave it unchanged.
  */
+
+#define sizeof_mbr(type, member) ({ sizeof(((type *)0)->member); })
+
+#define BOOT_PARAM_PRESERVE(struct_member)				\
+	{								\
+		.start = offsetof(struct boot_params, struct_member),	\
+		.len   = sizeof_mbr(struct boot_params, struct_member),	\
+	}
+
+struct boot_params_to_save {
+	unsigned int start;
+	unsigned int len;
+};
+
 static void sanitize_boot_params(struct boot_params *boot_params)
 {
 	/* 
@@ -35,21 +49,40 @@ static void sanitize_boot_params(struct boot_params *boot_params)
 	 * problems again.
 	 */
 	if (boot_params->sentinel) {
-		/* fields in boot_params are left uninitialized, clear them */
-		boot_params->acpi_rsdp_addr = 0;
-		memset(&boot_params->ext_ramdisk_image, 0,
-		       (char *)&boot_params->efi_info -
-			(char *)&boot_params->ext_ramdisk_image);
-		memset(&boot_params->kbd_status, 0,
-		       (char *)&boot_params->hdr -
-		       (char *)&boot_params->kbd_status);
-		memset(&boot_params->_pad7[0], 0,
-		       (char *)&boot_params->edd_mbr_sig_buffer[0] -
-			(char *)&boot_params->_pad7[0]);
-		memset(&boot_params->_pad8[0], 0,
-		       (char *)&boot_params->eddbuf[0] -
-			(char *)&boot_params->_pad8[0]);
-		memset(&boot_params->_pad9[0], 0, sizeof(boot_params->_pad9));
+		static struct boot_params scratch;
+		char *bp_base = (char *)boot_params;
+		char *save_base = (char *)&scratch;
+		int i;
+
+		const struct boot_params_to_save to_save[] = {
+			BOOT_PARAM_PRESERVE(screen_info),
+			BOOT_PARAM_PRESERVE(apm_bios_info),
+			BOOT_PARAM_PRESERVE(tboot_addr),
+			BOOT_PARAM_PRESERVE(ist_info),
+			BOOT_PARAM_PRESERVE(acpi_rsdp_addr),
+			BOOT_PARAM_PRESERVE(hd0_info),
+			BOOT_PARAM_PRESERVE(hd1_info),
+			BOOT_PARAM_PRESERVE(sys_desc_table),
+			BOOT_PARAM_PRESERVE(olpc_ofw_header),
+			BOOT_PARAM_PRESERVE(efi_info),
+			BOOT_PARAM_PRESERVE(alt_mem_k),
+			BOOT_PARAM_PRESERVE(scratch),
+			BOOT_PARAM_PRESERVE(e820_entries),
+			BOOT_PARAM_PRESERVE(eddbuf_entries),
+			BOOT_PARAM_PRESERVE(edd_mbr_sig_buf_entries),
+			BOOT_PARAM_PRESERVE(edd_mbr_sig_buffer),
+			BOOT_PARAM_PRESERVE(e820_table),
+			BOOT_PARAM_PRESERVE(eddbuf),
+		};
+
+		memset(&scratch, 0, sizeof(scratch));
+
+		for (i = 0; i < ARRAY_SIZE(to_save); i++) {
+			memcpy(save_base + to_save[i].start,
+			       bp_base + to_save[i].start, to_save[i].len);
+		}
+
+		memcpy(boot_params, save_base, sizeof(*boot_params));
 	}
 }
 

[tip:x86/boot] x86/boot: Save fields explicitly, zero out everything else

[tip-bot for John Hubbard]

Commit-ID:  a156cadef2fe445ac423670eace517b39a01ccd0
Gitweb:     https://git.kernel.org/tip/a156cadef2fe445ac423670eace517b39a01ccd0
Author:     John Hubbard <jhubbard@nvidia.com>
AuthorDate: Tue, 30 Jul 2019 22:46:27 -0700
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Wed, 7 Aug 2019 15:22:53 +0200

x86/boot: Save fields explicitly, zero out everything else

Recent gcc compilers (gcc 9.1) generate warnings about an out of bounds
memset, if the memset goes accross several fields of a struct. This
generated a couple of warnings on x86_64 builds in sanitize_boot_params().

Fix this by explicitly saving the fields in struct boot_params
that are intended to be preserved, and zeroing all the rest.

Suggested-by: Thomas Gleixner <tglx@linutronix.de>
Suggested-by: H. Peter Anvin <hpa@zytor.com>
Signed-off-by: John Hubbard <jhubbard@nvidia.com>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Link: https://lkml.kernel.org/r/20190731054627.5627-2-jhubbard@nvidia.com
---
 arch/x86/include/asm/bootparam_utils.h | 63 ++++++++++++++++++++++++++--------
 1 file changed, 48 insertions(+), 15 deletions(-)

diff --git a/arch/x86/include/asm/bootparam_utils.h b/arch/x86/include/asm/bootparam_utils.h
index 101eb944f13c..f5e90a849bca 100644
--- a/arch/x86/include/asm/bootparam_utils.h
+++ b/arch/x86/include/asm/bootparam_utils.h
@@ -18,6 +18,20 @@
  * Note: efi_info is commonly left uninitialized, but that field has a
  * private magic, so it is better to leave it unchanged.
  */
+
+#define sizeof_mbr(type, member) ({ sizeof(((type *)0)->member); })
+
+#define BOOT_PARAM_PRESERVE(struct_member)				\
+	{								\
+		.start = offsetof(struct boot_params, struct_member),	\
+		.len   = sizeof_mbr(struct boot_params, struct_member),	\
+	}
+
+struct boot_params_to_save {
+	unsigned int start;
+	unsigned int len;
+};
+
 static void sanitize_boot_params(struct boot_params *boot_params)
 {
 	/* 
@@ -35,21 +49,40 @@ static void sanitize_boot_params(struct boot_params *boot_params)
 	 * problems again.
 	 */
 	if (boot_params->sentinel) {
-		/* fields in boot_params are left uninitialized, clear them */
-		boot_params->acpi_rsdp_addr = 0;
-		memset(&boot_params->ext_ramdisk_image, 0,
-		       (char *)&boot_params->efi_info -
-			(char *)&boot_params->ext_ramdisk_image);
-		memset(&boot_params->kbd_status, 0,
-		       (char *)&boot_params->hdr -
-		       (char *)&boot_params->kbd_status);
-		memset(&boot_params->_pad7[0], 0,
-		       (char *)&boot_params->edd_mbr_sig_buffer[0] -
-			(char *)&boot_params->_pad7[0]);
-		memset(&boot_params->_pad8[0], 0,
-		       (char *)&boot_params->eddbuf[0] -
-			(char *)&boot_params->_pad8[0]);
-		memset(&boot_params->_pad9[0], 0, sizeof(boot_params->_pad9));
+		static struct boot_params scratch;
+		char *bp_base = (char *)boot_params;
+		char *save_base = (char *)&scratch;
+		int i;
+
+		const struct boot_params_to_save to_save[] = {
+			BOOT_PARAM_PRESERVE(screen_info),
+			BOOT_PARAM_PRESERVE(apm_bios_info),
+			BOOT_PARAM_PRESERVE(tboot_addr),
+			BOOT_PARAM_PRESERVE(ist_info),
+			BOOT_PARAM_PRESERVE(acpi_rsdp_addr),
+			BOOT_PARAM_PRESERVE(hd0_info),
+			BOOT_PARAM_PRESERVE(hd1_info),
+			BOOT_PARAM_PRESERVE(sys_desc_table),
+			BOOT_PARAM_PRESERVE(olpc_ofw_header),
+			BOOT_PARAM_PRESERVE(efi_info),
+			BOOT_PARAM_PRESERVE(alt_mem_k),
+			BOOT_PARAM_PRESERVE(scratch),
+			BOOT_PARAM_PRESERVE(e820_entries),
+			BOOT_PARAM_PRESERVE(eddbuf_entries),
+			BOOT_PARAM_PRESERVE(edd_mbr_sig_buf_entries),
+			BOOT_PARAM_PRESERVE(edd_mbr_sig_buffer),
+			BOOT_PARAM_PRESERVE(e820_table),
+			BOOT_PARAM_PRESERVE(eddbuf),
+		};
+
+		memset(&scratch, 0, sizeof(scratch));
+
+		for (i = 0; i < ARRAY_SIZE(to_save); i++) {
+			memcpy(save_base + to_save[i].start,
+			       bp_base + to_save[i].start, to_save[i].len);
+		}
+
+		memcpy(boot_params, save_base, sizeof(*boot_params));
 	}
 }
 

Re: [PATCH v2] x86/boot: save fields explicitly, zero out everything else

[John Hubbard]

On 8/7/19 4:41 AM, David Laight wrote:
> From: john.hubbard@gmail.com
>> Sent: 31 July 2019 06:46
...
>>  	if (boot_params->sentinel) {
>> -		/* fields in boot_params are left uninitialized, clear them */
>> -		boot_params->acpi_rsdp_addr = 0;
>> -		memset(&boot_params->ext_ramdisk_image, 0,
>> -		       (char *)&boot_params->efi_info -
>> -			(char *)&boot_params->ext_ramdisk_image);
>> -		memset(&boot_params->kbd_status, 0,
>> -		       (char *)&boot_params->hdr -
>> -		       (char *)&boot_params->kbd_status);
>> -		memset(&boot_params->_pad7[0], 0,
>> -		       (char *)&boot_params->edd_mbr_sig_buffer[0] -
>> -			(char *)&boot_params->_pad7[0]);
>> -		memset(&boot_params->_pad8[0], 0,
>> -		       (char *)&boot_params->eddbuf[0] -
>> -			(char *)&boot_params->_pad8[0]);
>> -		memset(&boot_params->_pad9[0], 0, sizeof(boot_params->_pad9));
> ...
> 
> How about replacing the above first using:
> #define zero_struct_fields(ptr, from, to) memset(&ptr->from, 0, (char *)&ptr->to - (char *)&ptr->from)
> 	zero_struct_fields(boot_params, ext_ramdisk_image, efi_info);
> 	...
> Which is absolutely identical to the original code.
> 
> The replacing the define with:
> 	#define so(s, m) offsetof(typeof(*s), m)
> 	#define zero_struct_fields(ptr, from, to) memset((char *)ptr + so(ptr, from), 0, so(ptr, to) - so(ptr, from))
> which gcc probably doesn't complain about, but should generate identical code again.
> There might be an existing define for so().
> 

Hi David,

There was discussion about that [1], but preference ending up being to
flip this around, in order to more closely match the original intent
of this function (zero out everything except for certain carefully
selected fields), and to therefore be more likely to keep working if 
fields are added. 


[1] https://lore.kernel.org/lkml/alpine.DEB.2.21.1907252358240.1791@nanos.tec.linutronix.de/

thanks,
-- 
John Hubbard
NVIDIA

Re: [PATCH v2] x86/boot: save fields explicitly, zero out everything else

[Chris Clayton]

On 31/07/2019 06:46, john.hubbard@gmail.com wrote:
> From: John Hubbard <jhubbard@nvidia.com>
> 
> Recent gcc compilers (gcc 9.1) generate warnings about an
> out of bounds memset, if you trying memset across several fields
> of a struct. This generated a couple of warnings on x86_64 builds.
> 
> Fix this by explicitly saving the fields in struct boot_params
> that are intended to be preserved, and zeroing all the rest.
> 

I applied John's patch below to v5.3-rc3-285-gecb095bff5d4 and have been running the resultant kernel for two days now,
including 7 or 8 cold starts and reboots. The warnings that were produced by gcc9 are no longer emitted and, other than
a pre-existing problem (no network after resume from suspend or hibernate which I will investigate and, if necessary,
report later today), the kernel has supported my typical day to day activities (building software, email, browsing,
listening to music, watching video) without problem.

Tested-by: Chris Clayton <chris2553@googlemail.com>

> Suggested-by: Thomas Gleixner <tglx@linutronix.de>
> Suggested-by: H. Peter Anvin <hpa@zytor.com>
> Signed-off-by: John Hubbard <jhubbard@nvidia.com>
> ---
>  arch/x86/include/asm/bootparam_utils.h | 62 +++++++++++++++++++-------
>  1 file changed, 47 insertions(+), 15 deletions(-)
> 
> diff --git a/arch/x86/include/asm/bootparam_utils.h b/arch/x86/include/asm/bootparam_utils.h
> index 101eb944f13c..514aee24b8de 100644
> --- a/arch/x86/include/asm/bootparam_utils.h
> +++ b/arch/x86/include/asm/bootparam_utils.h
> @@ -18,6 +18,20 @@
>   * Note: efi_info is commonly left uninitialized, but that field has a
>   * private magic, so it is better to leave it unchanged.
>   */
> +
> +#define sizeof_mbr(type, member) ({ sizeof(((type *)0)->member); })
> +
> +#define BOOT_PARAM_PRESERVE(struct_member)				\
> +	{								\
> +		.start = offsetof(struct boot_params, struct_member),	\
> +		.len   = sizeof_mbr(struct boot_params, struct_member),	\
> +	}
> +
> +struct boot_params_to_save {
> +	unsigned int start;
> +	unsigned int len;
> +};
> +
>  static void sanitize_boot_params(struct boot_params *boot_params)
>  {
>  	/* 
> @@ -35,21 +49,39 @@ static void sanitize_boot_params(struct boot_params *boot_params)
>  	 * problems again.
>  	 */
>  	if (boot_params->sentinel) {
> -		/* fields in boot_params are left uninitialized, clear them */
> -		boot_params->acpi_rsdp_addr = 0;
> -		memset(&boot_params->ext_ramdisk_image, 0,
> -		       (char *)&boot_params->efi_info -
> -			(char *)&boot_params->ext_ramdisk_image);
> -		memset(&boot_params->kbd_status, 0,
> -		       (char *)&boot_params->hdr -
> -		       (char *)&boot_params->kbd_status);
> -		memset(&boot_params->_pad7[0], 0,
> -		       (char *)&boot_params->edd_mbr_sig_buffer[0] -
> -			(char *)&boot_params->_pad7[0]);
> -		memset(&boot_params->_pad8[0], 0,
> -		       (char *)&boot_params->eddbuf[0] -
> -			(char *)&boot_params->_pad8[0]);
> -		memset(&boot_params->_pad9[0], 0, sizeof(boot_params->_pad9));
> +		static struct boot_params scratch;
> +		char *bp_base = (char *)boot_params;
> +		char *save_base = (char *)&scratch;
> +		int i;
> +
> +		const struct boot_params_to_save to_save[] = {
> +			BOOT_PARAM_PRESERVE(screen_info),
> +			BOOT_PARAM_PRESERVE(apm_bios_info),
> +			BOOT_PARAM_PRESERVE(tboot_addr),
> +			BOOT_PARAM_PRESERVE(ist_info),
> +			BOOT_PARAM_PRESERVE(acpi_rsdp_addr),
> +			BOOT_PARAM_PRESERVE(hd0_info),
> +			BOOT_PARAM_PRESERVE(hd1_info),
> +			BOOT_PARAM_PRESERVE(sys_desc_table),
> +			BOOT_PARAM_PRESERVE(olpc_ofw_header),
> +			BOOT_PARAM_PRESERVE(efi_info),
> +			BOOT_PARAM_PRESERVE(alt_mem_k),
> +			BOOT_PARAM_PRESERVE(scratch),
> +			BOOT_PARAM_PRESERVE(e820_entries),
> +			BOOT_PARAM_PRESERVE(eddbuf_entries),
> +			BOOT_PARAM_PRESERVE(edd_mbr_sig_buf_entries),
> +			BOOT_PARAM_PRESERVE(edd_mbr_sig_buffer),
> +			BOOT_PARAM_PRESERVE(e820_table),
> +			BOOT_PARAM_PRESERVE(eddbuf),
> +		};
> +
> +		memset(&scratch, 0, sizeof(scratch));
> +
> +		for (i = 0; i < ARRAY_SIZE(to_save); i++)
> +			memcpy(save_base + to_save[i].start,
> +			       bp_base + to_save[i].start, to_save[i].len);
> +
> +		memcpy(boot_params, save_base, sizeof(*boot_params));
>  	}
>  }
>  
> 

[tip:x86/urgent] x86/boot: Save fields explicitly, zero out everything else

[tip-bot for John Hubbard]

Commit-ID:  a90118c445cc7f07781de26a9684d4ec58bfcfd1
Gitweb:     https://git.kernel.org/tip/a90118c445cc7f07781de26a9684d4ec58bfcfd1
Author:     John Hubbard <jhubbard@nvidia.com>
AuthorDate: Tue, 30 Jul 2019 22:46:27 -0700
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Fri, 16 Aug 2019 14:20:00 +0200

x86/boot: Save fields explicitly, zero out everything else

Recent gcc compilers (gcc 9.1) generate warnings about an out of bounds
memset, if the memset goes accross several fields of a struct. This
generated a couple of warnings on x86_64 builds in sanitize_boot_params().

Fix this by explicitly saving the fields in struct boot_params
that are intended to be preserved, and zeroing all the rest.

[ tglx: Tagged for stable as it breaks the warning free build there as well ]

Suggested-by: Thomas Gleixner <tglx@linutronix.de>
Suggested-by: H. Peter Anvin <hpa@zytor.com>
Signed-off-by: John Hubbard <jhubbard@nvidia.com>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: stable@vger.kernel.org
Link: https://lkml.kernel.org/r/20190731054627.5627-2-jhubbard@nvidia.com

---
 arch/x86/include/asm/bootparam_utils.h | 63 ++++++++++++++++++++++++++--------
 1 file changed, 48 insertions(+), 15 deletions(-)

diff --git a/arch/x86/include/asm/bootparam_utils.h b/arch/x86/include/asm/bootparam_utils.h
index 101eb944f13c..f5e90a849bca 100644
--- a/arch/x86/include/asm/bootparam_utils.h
+++ b/arch/x86/include/asm/bootparam_utils.h
@@ -18,6 +18,20 @@
  * Note: efi_info is commonly left uninitialized, but that field has a
  * private magic, so it is better to leave it unchanged.
  */
+
+#define sizeof_mbr(type, member) ({ sizeof(((type *)0)->member); })
+
+#define BOOT_PARAM_PRESERVE(struct_member)				\
+	{								\
+		.start = offsetof(struct boot_params, struct_member),	\
+		.len   = sizeof_mbr(struct boot_params, struct_member),	\
+	}
+
+struct boot_params_to_save {
+	unsigned int start;
+	unsigned int len;
+};
+
 static void sanitize_boot_params(struct boot_params *boot_params)
 {
 	/* 
@@ -35,21 +49,40 @@ static void sanitize_boot_params(struct boot_params *boot_params)
 	 * problems again.
 	 */
 	if (boot_params->sentinel) {
-		/* fields in boot_params are left uninitialized, clear them */
-		boot_params->acpi_rsdp_addr = 0;
-		memset(&boot_params->ext_ramdisk_image, 0,
-		       (char *)&boot_params->efi_info -
-			(char *)&boot_params->ext_ramdisk_image);
-		memset(&boot_params->kbd_status, 0,
-		       (char *)&boot_params->hdr -
-		       (char *)&boot_params->kbd_status);
-		memset(&boot_params->_pad7[0], 0,
-		       (char *)&boot_params->edd_mbr_sig_buffer[0] -
-			(char *)&boot_params->_pad7[0]);
-		memset(&boot_params->_pad8[0], 0,
-		       (char *)&boot_params->eddbuf[0] -
-			(char *)&boot_params->_pad8[0]);
-		memset(&boot_params->_pad9[0], 0, sizeof(boot_params->_pad9));
+		static struct boot_params scratch;
+		char *bp_base = (char *)boot_params;
+		char *save_base = (char *)&scratch;
+		int i;
+
+		const struct boot_params_to_save to_save[] = {
+			BOOT_PARAM_PRESERVE(screen_info),
+			BOOT_PARAM_PRESERVE(apm_bios_info),
+			BOOT_PARAM_PRESERVE(tboot_addr),
+			BOOT_PARAM_PRESERVE(ist_info),
+			BOOT_PARAM_PRESERVE(acpi_rsdp_addr),
+			BOOT_PARAM_PRESERVE(hd0_info),
+			BOOT_PARAM_PRESERVE(hd1_info),
+			BOOT_PARAM_PRESERVE(sys_desc_table),
+			BOOT_PARAM_PRESERVE(olpc_ofw_header),
+			BOOT_PARAM_PRESERVE(efi_info),
+			BOOT_PARAM_PRESERVE(alt_mem_k),
+			BOOT_PARAM_PRESERVE(scratch),
+			BOOT_PARAM_PRESERVE(e820_entries),
+			BOOT_PARAM_PRESERVE(eddbuf_entries),
+			BOOT_PARAM_PRESERVE(edd_mbr_sig_buf_entries),
+			BOOT_PARAM_PRESERVE(edd_mbr_sig_buffer),
+			BOOT_PARAM_PRESERVE(e820_table),
+			BOOT_PARAM_PRESERVE(eddbuf),
+		};
+
+		memset(&scratch, 0, sizeof(scratch));
+
+		for (i = 0; i < ARRAY_SIZE(to_save); i++) {
+			memcpy(save_base + to_save[i].start,
+			       bp_base + to_save[i].start, to_save[i].len);
+		}
+
+		memcpy(boot_params, save_base, sizeof(*boot_params));
 	}
 }
 

[PATCH] x86/boot: Fix regression--secure boot info loss from bootparam sanitizing

[John S Gruber]

From: "John S. Gruber" <JohnSGruber@gmail.com>

commit a90118c445cc ("x86/boot: Save fields explicitly, zero out everything
else") now zeros the secure boot information passed by the boot loader or
by the kernel's efi handover mechanism.

Include boot-params.secure_boot in the preserve field list.

Signed-off-by: John S. Gruber <JohnSGruber@gmail.com>
---

I noted a change in my computers between running signed 5.3-rc4 and 5.3-rc6
with signed kernels using the efi handoff protocol with grub. The kernel
log message "Secure boot enabled" becomes "Secure boot could not be
determined". The efi_main function in arch/x86/boot/compressed/eboot.c sets
this field early but it is subsequently zeroed by the above referenced commit
in the file arch/x86/include/asm/bootparam_utils.h

Applies to 5.3-rc6.

 arch/x86/include/asm/bootparam_utils.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/x86/include/asm/bootparam_utils.h
b/arch/x86/include/asm/bootparam_utils.h
index 9e5f3c7..981fe92 100644
--- a/arch/x86/include/asm/bootparam_utils.h
+++ b/arch/x86/include/asm/bootparam_utils.h
@@ -70,6 +70,7 @@ static void sanitize_boot_params(struct boot_params
*boot_params)
 			BOOT_PARAM_PRESERVE(eddbuf_entries),
 			BOOT_PARAM_PRESERVE(edd_mbr_sig_buf_entries),
 			BOOT_PARAM_PRESERVE(edd_mbr_sig_buffer),
+			BOOT_PARAM_PRESERVE(secure_boot),
 			BOOT_PARAM_PRESERVE(hdr),
 			BOOT_PARAM_PRESERVE(e820_table),
 			BOOT_PARAM_PRESERVE(eddbuf),
-- 
2.7.4

Re: [PATCH] x86/boot: Fix regression--secure boot info loss from bootparam sanitizing

[John Hubbard]

On 9/1/19 8:38 AM, John S Gruber wrote:
> From: "John S. Gruber" <JohnSGruber@gmail.com>
> 
> commit a90118c445cc ("x86/boot: Save fields explicitly, zero out everything
> else") now zeros the secure boot information passed by the boot loader or
> by the kernel's efi handover mechanism.
> 
> Include boot-params.secure_boot in the preserve field list.
> 
> Signed-off-by: John S. Gruber <JohnSGruber@gmail.com>
> ---
> 
> I noted a change in my computers between running signed 5.3-rc4 and 5.3-rc6
> with signed kernels using the efi handoff protocol with grub. The kernel
> log message "Secure boot enabled" becomes "Secure boot could not be
> determined". The efi_main function in arch/x86/boot/compressed/eboot.c sets
> this field early but it is subsequently zeroed by the above referenced commit
> in the file arch/x86/include/asm/bootparam_utils.h
> 
> Applies to 5.3-rc6.
> 

Hi,

The fix itself looks good, so you can add:

     Reviewed-by: John Hubbard <jhubbard@nvidia.com>

...but note that the commit description should get a few tweaks:

1. Your description above is actually well-suited for the commit log,
so please add that in. Especially the symptoms are desirable to have
on record.

2. This should Cc: stable@vger.kernel.org, because the whole thing
made it into -stable and those kernels need this fix.

3. Also need a Fixes tag:

Fixes: commit a90118c445cc ("x86/boot: Save fields explicitly, zero out everything else")

thanks,
-- 
John Hubbard
NVIDIA

>   arch/x86/include/asm/bootparam_utils.h | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/arch/x86/include/asm/bootparam_utils.h
> b/arch/x86/include/asm/bootparam_utils.h
> index 9e5f3c7..981fe92 100644
> --- a/arch/x86/include/asm/bootparam_utils.h
> +++ b/arch/x86/include/asm/bootparam_utils.h
> @@ -70,6 +70,7 @@ static void sanitize_boot_params(struct boot_params
> *boot_params)
>   			BOOT_PARAM_PRESERVE(eddbuf_entries),
>   			BOOT_PARAM_PRESERVE(edd_mbr_sig_buf_entries),
>   			BOOT_PARAM_PRESERVE(edd_mbr_sig_buffer),
> +			BOOT_PARAM_PRESERVE(secure_boot),
>   			BOOT_PARAM_PRESERVE(hdr),
>   			BOOT_PARAM_PRESERVE(e820_table),
>   			BOOT_PARAM_PRESERVE(eddbuf),
> 

[PATCH V2] x86/boot: Fix regression--secure boot info loss from bootparam sanitizing

[John S Gruber]

From: "John S. Gruber" <JohnSGruber@gmail.com>

commit a90118c445cc ("x86/boot: Save fields explicitly, zero out everything
else") now zeros the secure boot information passed by the boot loader or
by the kernel's efi handover mechanism.  Include boot-params.secure_boot
in the preserve field list.

I noted a change in my computers between running signed 5.3-rc4 and 5.3-rc6
with signed kernels using the efi handoff protocol with grub. The kernel
log message "Secure boot enabled" becomes "Secure boot could not be
determined". The efi_main function in arch/x86/boot/compressed/eboot.c sets
this field early but it is subsequently zeroed by the above referenced
commit in the file arch/x86/include/asm/bootparam_utils.h

Fixes: commit a90118c445cc ("x86/boot: Save fields explicitly, zero
out everything else")
Signed-off-by: John S. Gruber <JohnSGruber@gmail.com>
---

Adjusted the patch for John Hubbard's comments.

 arch/x86/include/asm/bootparam_utils.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/x86/include/asm/bootparam_utils.h
b/arch/x86/include/asm/bootparam_utils.h
index 9e5f3c7..981fe92 100644
--- a/arch/x86/include/asm/bootparam_utils.h
+++ b/arch/x86/include/asm/bootparam_utils.h
@@ -70,6 +70,7 @@ static void sanitize_boot_params(struct boot_params
*boot_params)
 			BOOT_PARAM_PRESERVE(eddbuf_entries),
 			BOOT_PARAM_PRESERVE(edd_mbr_sig_buf_entries),
 			BOOT_PARAM_PRESERVE(edd_mbr_sig_buffer),
+			BOOT_PARAM_PRESERVE(secure_boot),
 			BOOT_PARAM_PRESERVE(hdr),
 			BOOT_PARAM_PRESERVE(e820_table),
 			BOOT_PARAM_PRESERVE(eddbuf),
-- 
2.7.4

Re: [PATCH V2] x86/boot: Fix regression--secure boot info loss from bootparam sanitizing

[Borislav Petkov]

On Mon, Sep 02, 2019 at 12:00:54AM +0200, John S Gruber wrote:
> From: "John S. Gruber" <JohnSGruber@gmail.com>
> 
> commit a90118c445cc ("x86/boot: Save fields explicitly, zero out everything
> else") now zeros the secure boot information passed by the boot loader or
> by the kernel's efi handover mechanism.  Include boot-params.secure_boot
> in the preserve field list.
> 
> I noted a change in my computers between running signed 5.3-rc4 and 5.3-rc6
> with signed kernels using the efi handoff protocol with grub. The kernel
> log message "Secure boot enabled" becomes "Secure boot could not be
> determined". The efi_main function in arch/x86/boot/compressed/eboot.c sets
> this field early but it is subsequently zeroed by the above referenced
> commit in the file arch/x86/include/asm/bootparam_utils.h
> 
> Fixes: commit a90118c445cc ("x86/boot: Save fields explicitly, zero
> out everything else")
> Signed-off-by: John S. Gruber <JohnSGruber@gmail.com>
> ---
> 
> Adjusted the patch for John Hubbard's comments.
> 
>  arch/x86/include/asm/bootparam_utils.h | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/arch/x86/include/asm/bootparam_utils.h
> b/arch/x86/include/asm/bootparam_utils.h
> index 9e5f3c7..981fe92 100644
> --- a/arch/x86/include/asm/bootparam_utils.h
> +++ b/arch/x86/include/asm/bootparam_utils.h
> @@ -70,6 +70,7 @@ static void sanitize_boot_params(struct boot_params
> *boot_params)

gmail has managed to chew this patch:

checking file arch/x86/include/asm/bootparam_utils.h
patch: **** malformed patch at line 48: *boot_params)

See: https://www.kernel.org/doc/html/latest/process/email-clients.html#gmail-web-gui

You might find a better client in there if you wanna send more patches
in the future.

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

[tip: x86/urgent] x86/boot: Preserve boot_params.secure_boot from sanitizing

[tip-bot2 for John S. Gruber]

The following commit has been merged into the x86/urgent branch of tip:

Commit-ID:     29d9a0b50736768f042752070e5cdf4e4d4c00df
Gitweb:        https://git.kernel.org/tip/29d9a0b50736768f042752070e5cdf4e4d4c00df
Author:        John S. Gruber <JohnSGruber@gmail.com>
AuthorDate:    Mon, 02 Sep 2019 00:00:54 +02:00
Committer:     Borislav Petkov <bp@suse.de>
CommitterDate: Mon, 02 Sep 2019 09:17:45 +02:00

x86/boot: Preserve boot_params.secure_boot from sanitizing

Commit

  a90118c445cc ("x86/boot: Save fields explicitly, zero out everything else")

now zeroes the secure boot setting information (enabled/disabled/...)
passed by the boot loader or by the kernel's EFI handover mechanism.

The problem manifests itself with signed kernels using the EFI handoff
protocol with grub and the kernel loses the information whether secure
boot is enabled in the firmware, i.e., the log message "Secure boot
enabled" becomes "Secure boot could not be determined".

efi_main() arch/x86/boot/compressed/eboot.c sets this field early but it
is subsequently zeroed by the above referenced commit.

Include boot_params.secure_boot in the preserve field list.

 [ bp: restructure commit message and massage. ]

Fixes: a90118c445cc ("x86/boot: Save fields explicitly, zero out everything else")
Signed-off-by: John S. Gruber <JohnSGruber@gmail.com>
Signed-off-by: Borislav Petkov <bp@suse.de>
Reviewed-by: John Hubbard <jhubbard@nvidia.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Juergen Gross <jgross@suse.com>
Cc: Mark Brown <broonie@kernel.org>
Cc: stable <stable@vger.kernel.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: x86-ml <x86@kernel.org>
Link: https://lkml.kernel.org/r/CAPotdmSPExAuQcy9iAHqX3js_fc4mMLQOTr5RBGvizyCOPcTQQ@mail.gmail.com
---
 arch/x86/include/asm/bootparam_utils.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/x86/include/asm/bootparam_utils.h b/arch/x86/include/asm/bootparam_utils.h
index 9e5f3c7..981fe92 100644
--- a/arch/x86/include/asm/bootparam_utils.h
+++ b/arch/x86/include/asm/bootparam_utils.h
@@ -70,6 +70,7 @@ static void sanitize_boot_params(struct boot_params *boot_params)
 			BOOT_PARAM_PRESERVE(eddbuf_entries),
 			BOOT_PARAM_PRESERVE(edd_mbr_sig_buf_entries),
 			BOOT_PARAM_PRESERVE(edd_mbr_sig_buffer),
+			BOOT_PARAM_PRESERVE(secure_boot),
 			BOOT_PARAM_PRESERVE(hdr),
 			BOOT_PARAM_PRESERVE(e820_table),
 			BOOT_PARAM_PRESERVE(eddbuf),

[PATCH] x86/boot: v4.4 stable and v4.9 stable boot failure due to dropped patch line

[John S Gruber]

This regards upstream commit a90118c445cc ("x86/boot: Save fields
explicitly, zero out everything else") application to linux-stable.

Its corresponding commits to the stable 4.4 and 4.9 trees didn't apply
correctly, probably due to a field name change (e820_table had been named
e820_map before 4.10).

On my desktop I'm unable to boot a signed kernel due to these commits.

Add e820_map (to replace e820_table) to the preserved fields so that the
E820 memory regions in boot_params can be accessed by the kernel after
boot_params has been sanitized.

Signed-off-by: John S Gruber <JohnSGruber@gmail.com>
Fixes: 41664b97f46e ("x86/boot: Save fields explicitly, zero out everything else")
Fixes: 4e478cb2ccdd ("x86/boot: Save fields explicitly, zero out everything else")
Link: https://lore.kernel.org/lkml/20190731054627.5627-2-jhubbard@nvidia.com/
---

I tested stable 4.14.145, 4.19.74, and 5.2.16 successfully under the same
circumstances. Only 4.4 and 4.9 are affected by this dropped line.

 arch/x86/include/asm/bootparam_utils.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/x86/include/asm/bootparam_utils.h b/arch/x86/include/asm/bootparam_utils.h
index 0232b5a..588d8fb 100644
--- a/arch/x86/include/asm/bootparam_utils.h
+++ b/arch/x86/include/asm/bootparam_utils.h
@@ -71,6 +71,7 @@ static void sanitize_boot_params(struct boot_params *boot_params)
 			BOOT_PARAM_PRESERVE(edd_mbr_sig_buf_entries),
 			BOOT_PARAM_PRESERVE(edd_mbr_sig_buffer),
 			BOOT_PARAM_PRESERVE(hdr),
+			BOOT_PARAM_PRESERVE(e820_map),
 			BOOT_PARAM_PRESERVE(eddbuf),
 		};
 
-- 
2.7.4

Re: [PATCH] x86/boot: v4.4 stable and v4.9 stable boot failure due to dropped patch line

[John Hubbard]

On 9/20/19 6:06 PM, John S Gruber wrote:
> This regards upstream commit a90118c445cc ("x86/boot: Save fields
> explicitly, zero out everything else") application to linux-stable.
> 
> Its corresponding commits to the stable 4.4 and 4.9 trees didn't apply
> correctly, probably due to a field name change (e820_table had been named
> e820_map before 4.10).
> 
> On my desktop I'm unable to boot a signed kernel due to these commits.
> 
> Add e820_map (to replace e820_table) to the preserved fields so that the
> E820 memory regions in boot_params can be accessed by the kernel after
> boot_params has been sanitized.
> 
> Signed-off-by: John S Gruber <JohnSGruber@gmail.com>
> Fixes: 41664b97f46e ("x86/boot: Save fields explicitly, zero out everything else")
> Fixes: 4e478cb2ccdd ("x86/boot: Save fields explicitly, zero out everything else")
> Link: https://lore.kernel.org/lkml/20190731054627.5627-2-jhubbard@nvidia.com/
> ---
> 
> I tested stable 4.14.145, 4.19.74, and 5.2.16 successfully under the same
> circumstances. Only 4.4 and 4.9 are affected by this dropped line.
> 
>  arch/x86/include/asm/bootparam_utils.h | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/arch/x86/include/asm/bootparam_utils.h b/arch/x86/include/asm/bootparam_utils.h
> index 0232b5a..588d8fb 100644
> --- a/arch/x86/include/asm/bootparam_utils.h
> +++ b/arch/x86/include/asm/bootparam_utils.h
> @@ -71,6 +71,7 @@ static void sanitize_boot_params(struct boot_params *boot_params)
>  			BOOT_PARAM_PRESERVE(edd_mbr_sig_buf_entries),
>  			BOOT_PARAM_PRESERVE(edd_mbr_sig_buffer),
>  			BOOT_PARAM_PRESERVE(hdr),
> +			BOOT_PARAM_PRESERVE(e820_map),
>  			BOOT_PARAM_PRESERVE(eddbuf),
>  		};
>  
> 

Reviewed-by: John Hubbard <jhubbard@nvidia.com>

Thanks for getting -stable figured out and fixed--this has not been smooth sailing!


thanks,
-- 
John Hubbard
NVIDIA

Re: [PATCH] x86/boot: v4.4 stable and v4.9 stable boot failure due to dropped patch line

[Greg Kroah-Hartman]

On Fri, Sep 20, 2019 at 06:38:23PM -0700, John Hubbard wrote:
> On 9/20/19 6:06 PM, John S Gruber wrote:
> > This regards upstream commit a90118c445cc ("x86/boot: Save fields
> > explicitly, zero out everything else") application to linux-stable.
> > 
> > Its corresponding commits to the stable 4.4 and 4.9 trees didn't apply
> > correctly, probably due to a field name change (e820_table had been named
> > e820_map before 4.10).
> > 
> > On my desktop I'm unable to boot a signed kernel due to these commits.
> > 
> > Add e820_map (to replace e820_table) to the preserved fields so that the
> > E820 memory regions in boot_params can be accessed by the kernel after
> > boot_params has been sanitized.
> > 
> > Signed-off-by: John S Gruber <JohnSGruber@gmail.com>
> > Fixes: 41664b97f46e ("x86/boot: Save fields explicitly, zero out everything else")
> > Fixes: 4e478cb2ccdd ("x86/boot: Save fields explicitly, zero out everything else")
> > Link: https://lore.kernel.org/lkml/20190731054627.5627-2-jhubbard@nvidia.com/
> > ---
> > 
> > I tested stable 4.14.145, 4.19.74, and 5.2.16 successfully under the same
> > circumstances. Only 4.4 and 4.9 are affected by this dropped line.
> > 
> >  arch/x86/include/asm/bootparam_utils.h | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/arch/x86/include/asm/bootparam_utils.h b/arch/x86/include/asm/bootparam_utils.h
> > index 0232b5a..588d8fb 100644
> > --- a/arch/x86/include/asm/bootparam_utils.h
> > +++ b/arch/x86/include/asm/bootparam_utils.h
> > @@ -71,6 +71,7 @@ static void sanitize_boot_params(struct boot_params *boot_params)
> >  			BOOT_PARAM_PRESERVE(edd_mbr_sig_buf_entries),
> >  			BOOT_PARAM_PRESERVE(edd_mbr_sig_buffer),
> >  			BOOT_PARAM_PRESERVE(hdr),
> > +			BOOT_PARAM_PRESERVE(e820_map),
> >  			BOOT_PARAM_PRESERVE(eddbuf),
> >  		};
> >  
> > 
> 
> Reviewed-by: John Hubbard <jhubbard@nvidia.com>
> 
> Thanks for getting -stable figured out and fixed--this has not been smooth sailing!

This change is already in the 4.4.y-rc and 4.9.y-rc releases that went
out two days ago for review.  So this should be fixed in the next
release.  If not, please let me know.

thanks,

greg k-h