linux-kernel - Re: [PATCH] efi-bgrt: Add error handling; inform the user when ignoring the BGRT

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>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20140731103110.GC15082@console-pimps.org>
Date:	Thu, 31 Jul 2014 11:31:10 +0100
From:	Matt Fleming <matt@...sole-pimps.org>
To:	Josh Triplett <josh@...htriplett.org>
Cc:	Matt Fleming <matt.fleming@...el.com>,
	Thomas Gleixner <tglx@...utronix.de>,
	Ingo Molnar <mingo@...hat.com>,
	"H. Peter Anvin" <hpa@...or.com>, x86@...nel.org,
	linux-efi@...r.kernel.org, linux-kernel@...r.kernel.org,
	Srihari Vijayaraghavan <linux.bug.reporting@...il.com>
Subject: Re: [PATCH] efi-bgrt: Add error handling; inform the user when
 ignoring the BGRT

On Wed, 30 Jul, at 12:23:32PM, Josh Triplett wrote:
> Gracefully handle failures to allocate memory for the image, which might
> be arbitrarily large.
> 
> efi_bgrt_init can fail in various ways as well, usually because the
> BIOS-provided BGRT structure does not match expectations.  Add
> appropriate error messages rather than failing silently.
> 
> Reported-by: Srihari Vijayaraghavan

This could really do with an email address appending.

> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=81321
> Signed-off-by: Josh Triplett <josh@...htriplett.org>
> ---
>  arch/x86/platform/efi/efi-bgrt.c | 36 ++++++++++++++++++++++++++++++------
>  1 file changed, 30 insertions(+), 6 deletions(-)

[...]

> @@ -61,14 +81,18 @@ void __init efi_bgrt_init(void)
>  		early_iounmap(image, sizeof(bmp_header));
>  	bgrt_image_size = bmp_header.size;
>  
> -	bgrt_image = kmalloc(bgrt_image_size, GFP_KERNEL);
> -	if (!bgrt_image)
> +	bgrt_image = kmalloc(bgrt_image_size, GFP_KERNEL | __GFP_NOWARN);
> +	if (!bgrt_image) {
> +		pr_err("Ignoring BGRT: failed to allocate memory for image (wanted %zu bytes)\n",
> +		       bgrt_image_size);
>  		return;

I'm not sure that using __GFP_NOWARN is the right thing to do here. If
for some reason we can't handle the BGRT image we should include checks
in the BGRT code, rather than relying on the page-allocation machinery
to save us.

Let's just use an explicit limit on the size of the BGRT image we're
willing to handle.

Either way, this hunk shouldn't be mixed in with the improved error
logging changes, it needs to be a separate patch.

-- 
Matt Fleming, Intel Open Source Technology Center
--
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/