Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.debian.kernel > #67585

Bug#963033: linux-image-arm64: kexec loses EFI system tables with Debian kernels

Cross-posted to 2 groups.

Show key headers only | View raw

[Multipart message — attachments visible in raw view] - view raw

Hello,

On Fri, Jun 26, 2020 at 04:34:28PM -0400, Gabriel Krisman Bertazi wrote:
> Hi,
> 
> This is introduced by a Debian specific patch
> features/all/lockdown/arm64-add-kernel-config-option-to-lock-down-when.patch
> 
> The following patch fixes it.

Note, I'm not an expert in the area this patch modifies, just some
general feedback.
 
> >8
> From: Gabriel Krisman Bertazi <krisman@collabora.com>
> Subject: [PATCH] arm64: Don't disable EFI boot mode on linux,uefi-secure-boot
>  table absence
> 
> The Debian specific out-of-tree kernel patch titled ("arm64: add kernel
> config option to lock down when in Secure Boot mode") introduces a
> regression for EFI-booted systems that don't have a
> "linux,uefi-secure-boot" FDT entry.
> 
> In these systems, when the table is not found, it causes the FDT
> function to error out and not return other UEFI tables, in particular
> the System Table, which makes the kernel think it is not running on EFI
> mode.
> 
> Instead, let the EFI mode boot continue with the correct System Table,
> and consider the efi secureboot mode as unknown.
> 
> This regression was found at least as early as the debian port to 5.4.19,
> but it still affects the most recent 5.7.6 debian kernel.
> 
> Signed-off-by: Gabriel Krisman Bertazi <krisman@collabora.com>
> ---
>  drivers/firmware/efi/arm-init.c  |  2 +-
>  drivers/firmware/efi/fdtparams.c | 18 +++++++++++-------
>  2 files changed, 12 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/firmware/efi/arm-init.c b/drivers/firmware/efi/arm-init.c
> index 78fcfbe3ddb9..fcb60320e77a 100644
> --- a/drivers/firmware/efi/arm-init.c
> +++ b/drivers/firmware/efi/arm-init.c
> @@ -206,7 +206,7 @@ void __init efi_init(void)
>  {
>  	struct efi_memory_map_data data;
>  	u64 efi_system_table;
> -	u32 secure_boot;
> +	u32 secure_boot = efi_secureboot_mode_unknown;
>  
>  	/* Grab UEFI information placed in FDT by stub */
>  	efi_system_table = efi_get_fdt_params(&data, &secure_boot);

I'd prefer to have the assignment in efi_get_fdt_params.

> diff --git a/drivers/firmware/efi/fdtparams.c b/drivers/firmware/efi/fdtparams.c
> index 152ca7cfccc9..78c36e582408 100644
> --- a/drivers/firmware/efi/fdtparams.c
> +++ b/drivers/firmware/efi/fdtparams.c
> @@ -96,13 +96,15 @@ u64 __init efi_get_fdt_params(struct efi_memory_map_data *mm, u32 *secure_boot)
>  	struct {
>  		void	*var;
>  		int	size;
> +		int	required;
> +
>  	} target[] = {
> -		[SYSTAB] = { &systab,		sizeof(systab) },
> -		[MMBASE] = { &mm->phys_map,	sizeof(mm->phys_map) },
> -		[MMSIZE] = { &mm->size,		sizeof(mm->size) },
> -		[DCSIZE] = { &mm->desc_size,	sizeof(mm->desc_size) },
> -		[DCVERS] = { &mm->desc_version,	sizeof(mm->desc_version) },
> -		[SBMODE] = { secure_boot,       sizeof(*secure_boot) },
> +		[SYSTAB] = {&systab,		sizeof(systab),	1},
> +		[MMBASE] = {&mm->phys_map,	sizeof(mm->phys_map), 1},
> +		[MMSIZE] = {&mm->size,		sizeof(mm->size), 1},
> +		[DCSIZE] = {&mm->desc_size,	sizeof(mm->desc_size), 1},
> +		[DCVERS] = {&mm->desc_version,	sizeof(mm->desc_version), 1},
> +		[SBMODE] = {secure_boot,	sizeof(*secure_boot), 0 },

Is the whitespace change intended here? I wonder if it would be easier
to drop this hunk and ...

>  	};
>  
>  	BUILD_BUG_ON(ARRAY_SIZE(target) != ARRAY_SIZE(name));
> @@ -125,8 +127,10 @@ u64 __init efi_get_fdt_params(struct efi_memory_map_data *mm, u32 *secure_boot)
>  				continue;
>  			if (!j)
>  				goto notfound;
> +
>  			pr_err("Can't find property '%s' in DT!\n", pname);
> -			return 0;
> +			if (target[j].required)
> +				return 0;

... do here

		if (j != SBMODE)
			return 0;

here.

Is it right that a missing linux,uefi-secure-boot property still results
in a pr_err?

I wonder what the upstream status of the broken patch is.

Best regards
Uwe

Back to linux.debian.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

Bug#963033: linux-image-arm64: kexec loses EFI system tables with Debian kernels Da Xue <da+debianbug@lessconfused.com> - 2020-06-18 07:00 +0200
  Bug#963033: linux-image-arm64: kexec loses EFI system tables with Debian kernels Gabriel Krisman Bertazi <krisman@collabora.com> - 2020-06-26 22:40 +0200
    Bug#963033: linux-image-arm64: kexec loses EFI system tables with Debian kernels Uwe Kleine-König <uwe@kleine-koenig.org> - 2020-07-22 21:30 +0200
      Bug#963033: linux-image-arm64: kexec loses EFI system tables with Debian kernels Gabriel Krisman Bertazi <krisman@collabora.com> - 2020-07-22 21:40 +0200
        Bug#963033: linux-image-arm64: kexec loses EFI system tables with Debian kernels Romain Perier <romain.perier@gmail.com> - 2020-07-24 15:20 +0200

csiph-web