In case a broken boot loader doesn't clear its struct boot_params clear
rsdp_addr in sanitize_boot_params().
This fixes commit e6e094e053af75 ("x86/acpi, x86/boot: Take RSDP
address from boot params if available") e.g. for the case of a boot via
systemd-boot.
Fixes: e6e094e053af75 ("x86/acpi, x86/boot: Take RSDP address from boot params if available")
Reported-by: Gunnar Krueger <[email protected]>
Tested-by: Gunnar Krueger <[email protected]>
Signed-off-by: Juergen Gross <[email protected]>
---
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 a07ffd23e4dd..f6f6ef436599 100644
--- a/arch/x86/include/asm/bootparam_utils.h
+++ b/arch/x86/include/asm/bootparam_utils.h
@@ -36,6 +36,7 @@ static void sanitize_boot_params(struct boot_params *boot_params)
*/
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);
--
2.16.4
Commit-ID: 182ddd16194cd082f25fa1b063dae3c7c5cce384
Gitweb: https://git.kernel.org/tip/182ddd16194cd082f25fa1b063dae3c7c5cce384
Author: Juergen Gross <[email protected]>
AuthorDate: Mon, 3 Dec 2018 11:38:11 +0100
Committer: Ingo Molnar <[email protected]>
CommitDate: Mon, 3 Dec 2018 11:56:37 +0100
x86/boot: Clear RSDP address in boot_params for broken loaders
Gunnar Krueger reported a systemd-boot failure and bisected it down to:
e6e094e053af75 ("x86/acpi, x86/boot: Take RSDP address from boot params if available")
In case a broken boot loader doesn't clear its 'struct boot_params', clear
rsdp_addr in sanitize_boot_params().
Reported-by: Gunnar Krueger <[email protected]>
Tested-by: Gunnar Krueger <[email protected]>
Signed-off-by: Juergen Gross <[email protected]>
Cc: H. Peter Anvin <[email protected]>
Cc: Linus Torvalds <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: [email protected]
Cc: [email protected]
Fixes: e6e094e053af75 ("x86/acpi, x86/boot: Take RSDP address from boot params if available")
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Ingo Molnar <[email protected]>
---
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 a07ffd23e4dd..f6f6ef436599 100644
--- a/arch/x86/include/asm/bootparam_utils.h
+++ b/arch/x86/include/asm/bootparam_utils.h
@@ -36,6 +36,7 @@ static void sanitize_boot_params(struct boot_params *boot_params)
*/
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);
On December 3, 2018 2:38:11 AM PST, Juergen Gross <[email protected]> wrote:
>In case a broken boot loader doesn't clear its struct boot_params clear
>rsdp_addr in sanitize_boot_params().
>
>This fixes commit e6e094e053af75 ("x86/acpi, x86/boot: Take RSDP
>address from boot params if available") e.g. for the case of a boot via
>systemd-boot.
>
>Fixes: e6e094e053af75 ("x86/acpi, x86/boot: Take RSDP address from boot
>params if available")
>Reported-by: Gunnar Krueger <[email protected]>
>Tested-by: Gunnar Krueger <[email protected]>
>Signed-off-by: Juergen Gross <[email protected]>
>---
> 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 a07ffd23e4dd..f6f6ef436599 100644
>--- a/arch/x86/include/asm/bootparam_utils.h
>+++ b/arch/x86/include/asm/bootparam_utils.h
>@@ -36,6 +36,7 @@ static void sanitize_boot_params(struct boot_params
>*boot_params)
> */
> 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);
Isn't this already covered by the memset()? If not, we should extend the memset() to maximal coverage.
--
Sent from my Android device with K-9 Mail. Please excuse my brevity.
On 04/12/2018 00:07, [email protected] wrote:
> On December 3, 2018 2:38:11 AM PST, Juergen Gross <[email protected]> wrote:
>> In case a broken boot loader doesn't clear its struct boot_params clear
>> rsdp_addr in sanitize_boot_params().
>>
>> This fixes commit e6e094e053af75 ("x86/acpi, x86/boot: Take RSDP
>> address from boot params if available") e.g. for the case of a boot via
>> systemd-boot.
>>
>> Fixes: e6e094e053af75 ("x86/acpi, x86/boot: Take RSDP address from boot
>> params if available")
>> Reported-by: Gunnar Krueger <[email protected]>
>> Tested-by: Gunnar Krueger <[email protected]>
>> Signed-off-by: Juergen Gross <[email protected]>
>> ---
>> 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 a07ffd23e4dd..f6f6ef436599 100644
>> --- a/arch/x86/include/asm/bootparam_utils.h
>> +++ b/arch/x86/include/asm/bootparam_utils.h
>> @@ -36,6 +36,7 @@ static void sanitize_boot_params(struct boot_params
>> *boot_params)
>> */
>> 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);
>
> Isn't this already covered by the memset()? If not, we should extend the memset() to maximal coverage.
I'd like to send a followup patch doing that. And I'd like to not only
test sentinel for being non-zero, but all padding fields as well. This
should be 4.21 material, though.
Juergen
On 12/3/18 9:32 PM, Juergen Gross wrote:
>
> I'd like to send a followup patch doing that. And I'd like to not only
> test sentinel for being non-zero, but all padding fields as well. This
> should be 4.21 material, though.
>
No, you can't do that. That breaks backwards compatibility.
-hpa
On 04/12/2018 06:49, H. Peter Anvin wrote:
> On 12/3/18 9:32 PM, Juergen Gross wrote:
>>
>> I'd like to send a followup patch doing that. And I'd like to not only
>> test sentinel for being non-zero, but all padding fields as well. This
>> should be 4.21 material, though.
>>
>
> No, you can't do that. That breaks backwards compatibility.
So you are speaking about paddings which are at places where there used
to be some information? Shouldn't those be named "_res*"?
Recycling such paddings with some useful information seems to be rather
dangerous then.
I'd like to have at least some idea which boot loader is not passing a
clean struct boot_params. So I think we should at least have some debug
or info messages telling us which paddings are not zero initially to be
able to either fix the boot loader or switch from _pad* to _res* naming.
Juergen