linux-kernel - Re: [tip:x86/urgent] x86/efi: Only load initrd above 4g on second try

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [day] [month] [year] [list]

Date:	Tue, 09 Sep 2014 17:34:21 +0200
From:	Laszlo Ersek <lersek@...hat.com>
To:	linux-kernel@...r.kernel.org, mingo@...nel.org, hpa@...or.com,
	calvin.walton@...stin.ca, yinghai@...nel.org, grawity@...il.com,
	anders@...rgestorm.se, matt.fleming@...el.com, harald@...hat.com,
	tglx@...utronix.de, linux-tip-commits@...r.kernel.org
Subject: Re: [tip:x86/urgent] x86/efi: Only load initrd above 4g on second
 try

On 09/09/14 17:18, tip-bot for Yinghai Lu wrote:
> Commit-ID:  47226ad4f4cfd1e91ded7f2ec42f83ff1c624663
> Gitweb:     http://git.kernel.org/tip/47226ad4f4cfd1e91ded7f2ec42f83ff1c624663
> Author:     Yinghai Lu <yinghai@...nel.org>
> AuthorDate: Wed, 3 Sep 2014 21:50:07 -0700
> Committer:  Matt Fleming <matt.fleming@...el.com>
> CommitDate: Mon, 8 Sep 2014 20:52:02 +0100
> 
> x86/efi: Only load initrd above 4g on second try
> 
> Mantas found that after commit 4bf7111f5016 ("x86/efi: Support initrd
> loaded above 4G"), the kernel freezes at the earliest possible moment
> when trying to boot via UEFI on Asus laptop.
> 
> Revert to old way to load initrd under 4G on first try, second try will
> use above 4G buffer when initrd is too big and does not fit under 4G.
> 
> [ The cause of the freeze appears to be a firmware bug when reading
>   file data into buffers above 4GB, though the exact reason is unknown.
>   Mantas reports that the hang can be avoid if the file size is a
>   multiple of 512 bytes, but I've seen some ASUS firmware simply
>   corrupting the file data rather than freezing.
> 
>   Laszlo fixed an issue in the upstream EDK2 DiskIO code in Aug 2013
>   which may possibly be related, commit 4e39b75e ("MdeModulePkg/DiskIoDxe:
>   fix source/destination pointer of overrun transfer").
> 
>   Whatever the cause, it's unlikely that a fix will be forthcoming
>   from the vendor, hence the workaround - Matt ]
> 
> Cc: Laszlo Ersek <lersek@...hat.com>
> Reported-by: Mantas Mikulėnas <grawity@...il.com>
> Reported-by: Harald Hoyer <harald@...hat.com>
> Tested-by: Anders Darander <anders@...rgestorm.se>
> Tested-by: Calvin Walton <calvin.walton@...stin.ca>
> Signed-off-by: Yinghai Lu <yinghai@...nel.org>
> Signed-off-by: Matt Fleming <matt.fleming@...el.com>
> ---
>  arch/x86/boot/compressed/eboot.c | 18 +++++++++++-------
>  1 file changed, 11 insertions(+), 7 deletions(-)
> 
> diff --git a/arch/x86/boot/compressed/eboot.c b/arch/x86/boot/compressed/eboot.c
> index f277184..dca9842 100644
> --- a/arch/x86/boot/compressed/eboot.c
> +++ b/arch/x86/boot/compressed/eboot.c
> @@ -1032,7 +1032,6 @@ struct boot_params *make_boot_params(struct efi_config *c)
>  	int i;
>  	unsigned long ramdisk_addr;
>  	unsigned long ramdisk_size;
> -	unsigned long initrd_addr_max;
>  
>  	efi_early = c;
>  	sys_table = (efi_system_table_t *)(unsigned long)efi_early->table;
> @@ -1095,15 +1094,20 @@ struct boot_params *make_boot_params(struct efi_config *c)
>  
>  	memset(sdt, 0, sizeof(*sdt));
>  
> -	if (hdr->xloadflags & XLF_CAN_BE_LOADED_ABOVE_4G)
> -		initrd_addr_max = -1UL;
> -	else
> -		initrd_addr_max = hdr->initrd_addr_max;
> -
>  	status = handle_cmdline_files(sys_table, image,
>  				      (char *)(unsigned long)hdr->cmd_line_ptr,
> -				      "initrd=", initrd_addr_max,
> +				      "initrd=", hdr->initrd_addr_max,
>  				      &ramdisk_addr, &ramdisk_size);
> +
> +	if (status != EFI_SUCCESS &&
> +	    hdr->xloadflags & XLF_CAN_BE_LOADED_ABOVE_4G) {
> +		efi_printk(sys_table, "Trying to load files to higher address\n");
> +		status = handle_cmdline_files(sys_table, image,
> +				      (char *)(unsigned long)hdr->cmd_line_ptr,
> +				      "initrd=", -1UL,
> +				      &ramdisk_addr, &ramdisk_size);
> +	}
> +
>  	if (status != EFI_SUCCESS)
>  		goto fail2;
>  	hdr->ramdisk_image = ramdisk_addr & 0xffffffff;
> 

Assuming that handle_cmdline_files() cleans up things internally on a
failed attempt (no memory leaked etc), this seems sane to me. (All I
know about this code is visible in the above context.)

Acked-by: Laszlo Ersek <lersek@...hat.com>

Thanks
Laszlo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/