[PATCH] x86/EFI: check table header length in efi_bgrt_init()

[PATCH] x86/EFI: check table header length in efi_bgrt_init()

[Jan Beulich]

Header length should be validated for all ACPI tables before accessing
any non-header field.

Signed-off-by: Jan Beulich <jbeulich-IBi9RG/b67k@public.gmane.org>

---
 arch/x86/platform/efi/efi-bgrt.c |    2 ++
 1 file changed, 2 insertions(+)

--- 3.7-rc4/arch/x86/platform/efi/efi-bgrt.c
+++ 3.7-rc4-x86-EFI-BGRT-checks/arch/x86/platform/efi/efi-bgrt.c
@@ -39,6 +39,8 @@ void efi_bgrt_init(void)
 	if (ACPI_FAILURE(status))
 		return;
 
+	if (bgrt_tab->header.length < sizeof(*bgrt_tab))
+		return;
 	if (bgrt_tab->version != 1)
 		return;
 	if (bgrt_tab->image_type != 0 || !bgrt_tab->image_address)

Re: [PATCH] x86/EFI: check table header length in efi_bgrt_init()

[Josh Triplett]

On Wed, Nov 07, 2012 at 04:46:08PM +0000, Jan Beulich wrote:
> Header length should be validated for all ACPI tables before accessing
> any non-header field.
> 
> Signed-off-by: Jan Beulich <jbeulich-IBi9RG/b67k@public.gmane.org>

Reviewed-by: Josh Triplett <josh-iaAMLnmF4UmaiuxdJuQwMA@public.gmane.org>

> ---
>  arch/x86/platform/efi/efi-bgrt.c |    2 ++
>  1 file changed, 2 insertions(+)
> 
> --- 3.7-rc4/arch/x86/platform/efi/efi-bgrt.c
> +++ 3.7-rc4-x86-EFI-BGRT-checks/arch/x86/platform/efi/efi-bgrt.c
> @@ -39,6 +39,8 @@ void efi_bgrt_init(void)
>  	if (ACPI_FAILURE(status))
>  		return;
>  
> +	if (bgrt_tab->header.length < sizeof(*bgrt_tab))
> +		return;
>  	if (bgrt_tab->version != 1)
>  		return;
>  	if (bgrt_tab->image_type != 0 || !bgrt_tab->image_address)
> 
> 
> 

Re: [PATCH] x86/EFI: check table header length in efi_bgrt_init()

[Matt Fleming]

On Wed, 2012-11-07 at 16:46 +0000, Jan Beulich wrote:
> Header length should be validated for all ACPI tables before accessing
> any non-header field.
> 
> Signed-off-by: Jan Beulich <jbeulich-IBi9RG/b67k@public.gmane.org>
> 
> ---
>  arch/x86/platform/efi/efi-bgrt.c |    2 ++
>  1 file changed, 2 insertions(+)
> 
> --- 3.7-rc4/arch/x86/platform/efi/efi-bgrt.c
> +++ 3.7-rc4-x86-EFI-BGRT-checks/arch/x86/platform/efi/efi-bgrt.c
> @@ -39,6 +39,8 @@ void efi_bgrt_init(void)
>  	if (ACPI_FAILURE(status))
>  		return;
>  
> +	if (bgrt_tab->header.length < sizeof(*bgrt_tab))
> +		return;
>  	if (bgrt_tab->version != 1)
>  		return;
>  	if (bgrt_tab->image_type != 0 || !bgrt_tab->image_address)

Guys, do you want me to take this into the efi tree? Jan, have you see
machines that actually trip up without this check? I'm trying to gauge
the urgency of this patch.

-- 
Matt Fleming, Intel Open Source Technology Center

Re: [PATCH] x86/EFI: check table header length in efi_bgrt_init()

[Josh Triplett]

On Tue, Nov 13, 2012 at 08:08:15PM +0000, Matt Fleming wrote:
> On Wed, 2012-11-07 at 16:46 +0000, Jan Beulich wrote:
> > Header length should be validated for all ACPI tables before accessing
> > any non-header field.
> > 
> > Signed-off-by: Jan Beulich <jbeulich-IBi9RG/b67k@public.gmane.org>
> > 
> > ---
> >  arch/x86/platform/efi/efi-bgrt.c |    2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > --- 3.7-rc4/arch/x86/platform/efi/efi-bgrt.c
> > +++ 3.7-rc4-x86-EFI-BGRT-checks/arch/x86/platform/efi/efi-bgrt.c
> > @@ -39,6 +39,8 @@ void efi_bgrt_init(void)
> >  	if (ACPI_FAILURE(status))
> >  		return;
> >  
> > +	if (bgrt_tab->header.length < sizeof(*bgrt_tab))
> > +		return;
> >  	if (bgrt_tab->version != 1)
> >  		return;
> >  	if (bgrt_tab->image_type != 0 || !bgrt_tab->image_address)
> 
> Guys, do you want me to take this into the efi tree? Jan, have you see
> machines that actually trip up without this check? I'm trying to gauge
> the urgency of this patch.

I'd expect this to either go through efi or through tip, whichever you
prefer.  If Jan has a system that would fail this check, then this patch
seems urgent; otherwise, it could potentially wait until the merge
window, but as a bugfix it doesn't necessarily need to.

- Josh Triplett

Re: [PATCH] x86/EFI: check table header length in efi_bgrt_init()

[Jan Beulich]

>>> On 13.11.12 at 21:08, Matt Fleming <matt-HNK1S37rvNbeXh+fF434Mdi2O/JbrIOy@public.gmane.org> wrote:
> On Wed, 2012-11-07 at 16:46 +0000, Jan Beulich wrote:
>> Header length should be validated for all ACPI tables before accessing
>> any non-header field.
>> 
>> Signed-off-by: Jan Beulich <jbeulich-IBi9RG/b67k@public.gmane.org>
>> 
>> ---
>>  arch/x86/platform/efi/efi-bgrt.c |    2 ++
>>  1 file changed, 2 insertions(+)
>> 
>> --- 3.7-rc4/arch/x86/platform/efi/efi-bgrt.c
>> +++ 3.7-rc4-x86-EFI-BGRT-checks/arch/x86/platform/efi/efi-bgrt.c
>> @@ -39,6 +39,8 @@ void efi_bgrt_init(void)
>>  	if (ACPI_FAILURE(status))
>>  		return;
>>  
>> +	if (bgrt_tab->header.length < sizeof(*bgrt_tab))
>> +		return;
>>  	if (bgrt_tab->version != 1)
>>  		return;
>>  	if (bgrt_tab->image_type != 0 || !bgrt_tab->image_address)
> 
> Guys, do you want me to take this into the efi tree? Jan, have you see
> machines that actually trip up without this check? I'm trying to gauge
> the urgency of this patch.

No, I haven't. I just spotted that omission in the context of the
apparent lack of checking of the valid flag (which meanwhile was
explained to me as being intentional).

Jan

Re: [PATCH] x86/EFI: check table header length in efi_bgrt_init()

[Matt Fleming]

On Wed, 2012-11-14 at 09:10 +0000, Jan Beulich wrote:
> No, I haven't. I just spotted that omission in the context of the
> apparent lack of checking of the valid flag (which meanwhile was
> explained to me as being intentional).

OK, thanks for the clarification Jan.

I don't have anything else queued up for the next merge window, so I
will probably submit this next week after -rc6 as I'm awaiting feedback
for another bug fix and that will allow me to batch the two together.

Alternatively, tip folks, feel free to take this directly.

-- 
Matt Fleming, Intel Open Source Technology Center

[tip:core/efi] x86, efi: Check table header length in efi_bgrt_init()

[tip-bot for Jan Beulich]

Commit-ID:  5d6d578c170bb280db5d4779f666e456f4f82ec5
Gitweb:     http://git.kernel.org/tip/5d6d578c170bb280db5d4779f666e456f4f82ec5
Author:     Jan Beulich <JBeulich@suse.com>
AuthorDate: Wed, 7 Nov 2012 16:46:08 +0000
Committer:  H. Peter Anvin <hpa@linux.intel.com>
CommitDate: Wed, 14 Nov 2012 08:49:34 -0800

x86, efi: Check table header length in efi_bgrt_init()

Header length should be validated for all ACPI tables before accessing
any non-header field.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Link: http://lkml.kernel.org/r/509A9E6002000078000A7079@nat28.tlf.novell.com
Acked-by: Matt Fleming <matt.fleming@intel.com>
Signed-off-by: H. Peter Anvin <hpa@linux.intel.com>
---
 arch/x86/platform/efi/efi-bgrt.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/arch/x86/platform/efi/efi-bgrt.c b/arch/x86/platform/efi/efi-bgrt.c
index f6a0c1b..d9c1b95 100644
--- a/arch/x86/platform/efi/efi-bgrt.c
+++ b/arch/x86/platform/efi/efi-bgrt.c
@@ -39,6 +39,8 @@ void efi_bgrt_init(void)
 	if (ACPI_FAILURE(status))
 		return;
 
+	if (bgrt_tab->header.length < sizeof(*bgrt_tab))
+		return;
 	if (bgrt_tab->version != 1)
 		return;
 	if (bgrt_tab->image_type != 0 || !bgrt_tab->image_address)