linux-kernel - Re: "54b52d87268034859191d671505bb1cfce6bd74d - x86/efi: Build our own EFI services pointer table" breaks boot on thinkpad t440s

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

Message-ID: <20140411074428.GA22042@console-pimps.org>
Date:	Fri, 11 Apr 2014 08:44:28 +0100
From:	Matt Fleming <matt@...sole-pimps.org>
To:	Ingo Molnar <mingo@...nel.org>
Cc:	Koen Kooi <koen@...inion.thruhere.net>,
	Matt Fleming <matt.fleming@...el.com>,
	"H. Peter Anvin" <hpa@...or.com>,
	Thomas Gleixner <tglx@...utronix.de>,
	Ingo Molnar <mingo@...hat.com>, x86@...nel.org,
	Kees Cook <keescook@...omium.org>,
	Zhang Yanfei <zhangyanfei@...fujitsu.com>,
	linux-efi@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: "54b52d87268034859191d671505bb1cfce6bd74d - x86/efi: Build our
 own EFI services pointer table" breaks boot on thinkpad t440s

On Fri, 11 Apr, at 09:20:44AM, Ingo Molnar wrote:
> 
> Might be prudent to do the same in __file_size32(), instead of 
> truncating silently, especially is that function too has a u64 output 
> AFAICS.
 
This change isn't required for __file_size32() because we only use that
function if the firmware is 32-bit. The signature of
EFI_FILE_PROTOCOL.GetInfo() looks like this,

  EFI_STATUS (*EFI_FILE_GET_INFO) (
	EFI_FILE_PROTOCOL *This,
	EFI_GUID 	  *InformationType,
	UINTN		  *BufferSize,
	void		  *Buffer
  );

UINTN translates to unsigned long, so for 32-bit firmware is u32. The
firmware will never write 64-bits to &info_sz in __file_size32().

> Also, while reviewing the file I noticed that there's "u32 fb_base", 
> which is recovered like:
> 
>                 status = __gop_query64(gop64, &info, &size, &fb_base);
> 
> but ->frame_buffer_base is u64. Is it always guaranteed u32?

Good catch. ->frame_buffer_base isn't always u32, but we only have u32
bits in which to store it (struct screen_info.lfb_base), so we
implicitly truncate it,

static efi_status_t
__gop_query64(struct efi_graphics_output_protocol_64 *gop64,

[...]

        *fb_base = mode->frame_buffer_base;

But you raise a good point - it would probably make more sense to
complain loudly if we get an address above 0xffffffff.

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