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]

Date:	Fri, 1 Aug 2014 09:11:54 -0700
From:	Josh Triplett <josh@...htriplett.org>
To:	Matt Fleming <matt@...sole-pimps.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>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Matthew Garrett <mjg59@...f.ucam.org>
Subject: Re: [PATCH] efi-bgrt: Add error handling; inform the user when
 ignoring the BGRT

On Fri, Aug 01, 2014 at 10:19:49AM +0100, Matt Fleming wrote:
> (Including akpm, the __GFP_NOWARN police)

Andrew suggested __GFP_NOWARN here in the first place.

> On Thu, 31 Jul, at 09:11:33AM, Josh Triplett wrote:
> > 
> > I started to add an explicit limit, but any reasonable limit (large
> > enough for modern screens) would be large enough that there's still a
> > non-trivial possibility of allocation failure.  And I think it makes
> > sense for BGRT image allocation to be non-fatal and minimally noisy
> > (just a single-line error, not a scary-looking allocation warning),
> > considering the highly optional and cosmetic nature of BGRT.  So, I
> > believe __GFP_NOWARN makes sense.
> 
> Yes, I agree that we don't want to trigger the page allocator warning,
> but I don't agree that passing __GFP_NOWARN is OK, which is why I'm
> advocating some size limit checks.
> 
> We need to distinguish between "Your BGRT image size is huge, and
> assumed buggy" and "Your BGRT looks valid, but we ran out of memory".
> 
> We've already got enough problems with the EFI code because we silently
> paper over bugs, and using the page allocator's failure path as a way to
> check for buggy BGRT images just doesn't make any sense to me at all.
> 
> If we get the limit wrong, it's not the end of the world, we can change
> it later, but it's a safe bet that if the firmware engineers start
> seeing "BGRT is buggy" in the kernel log they're going to start a
> dialogue with us.

The original bug report was about an allocation failure for a fairly
reasonable BGRT size.  We can certainly prohibit absurdly huge ones (for
instance, bigger than the maximum likely screen resolution times 4 bytes
per pixel), but allocation failures may well occur for smaller sizes,
and I don't think we want to spew a massive warning for that either.

- Josh Triplett
--
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/