Re: [PATCH 2/4] efi/x86: move efi bgrt init code to early init code

From	Dave Young <dyoung@redhat.com>
Newsgroups	linux.kernel
Subject	Re: [PATCH 2/4] efi/x86: move efi bgrt init code to early init code
Date	2017-01-13 03:30 +0100
Message-ID	<sYZMR-3st-3@gated-at.bofh.it> (permalink)
References	<sYKb7-2kU-3@gated-at.bofh.it> <sYKb7-2kU-7@gated-at.bofh.it> <sYMcV-3w1-1@gated-at.bofh.it> <sYVge-Ak-7@gated-at.bofh.it> <sYWOZ-1DN-13@gated-at.bofh.it>
Organization	linux.* mail to news gateway

Show all headers | View raw

On 01/13/17 at 12:11am, Nicolai Stange wrote:
> On Fri, Jan 13 2017, Dave Young wrote:
> 
> > On 01/12/17 at 12:54pm, Nicolai Stange wrote:
> >> On Thu, Jan 12 2017, Dave Young wrote:
> >> 
> >> > -void __init efi_bgrt_init(void)
> >> > +void __init efi_bgrt_init(struct acpi_table_header *table)
> >> >  {
> >> > -	acpi_status status;
> >> >  	void *image;
> >> >  	struct bmp_header bmp_header;
> >> >  
> >> >  	if (acpi_disabled)
> >> >  		return;
> >> >  
> >> > -	status = acpi_get_table("BGRT", 0,
> >> > -	                        (struct acpi_table_header **)&bgrt_tab);
> >> > -	if (ACPI_FAILURE(status))
> >> > -		return;
> >> 
> >> 
> >> Not sure, but wouldn't it be safer to reverse the order of this assignment
> >> 
> >> > +	bgrt_tab = *(struct acpi_table_bgrt *)table;
> >
> > Nicolai, sorry, I'm not sure I understand the comment, is it about above line?
> > Could you elaborate a bit?
> >
> >> 
> >> and this length check
> >> 
> >
> > I also do not get this :(
> 
> Ah sorry, my point is this: the length check should perhaps be made
> before doing the assignment to bgrt_tab because otherwise, we might end
> up reading from invalid memory.
> 
> I.e. if (struct acpi_table_bgrt *)table->length < sizeof(bgrt_tab), then
> 
>   bgrt_tab = *(struct acpi_table_bgrt *)table;
> 
> would read past the table's end.
> 
> I'm not sure whether this is a real problem though -- that is, whether
> this read could ever hit some unmapped memory.

Nicolai, thanks for the explanation. It make sense to move it to even later
at the end of the function.

Thanks
Dave

Thread

[PATCH 2/4] efi/x86: move efi bgrt init code to early init code Dave Young <dyoung@redhat.com> - 2017-01-12 10:50 +0100
  Re: [PATCH 2/4] efi/x86: move efi bgrt init code to early init code Dave Young <dyoung@redhat.com> - 2017-01-12 11:00 +0100
  Re: [PATCH 2/4] efi/x86: move efi bgrt init code to early init code Nicolai Stange <nicstange@gmail.com> - 2017-01-12 13:00 +0100
    Re: [PATCH 2/4] efi/x86: move efi bgrt init code to early init code Dave Young <dyoung@redhat.com> - 2017-01-12 22:40 +0100
      Re: [PATCH 2/4] efi/x86: move efi bgrt init code to early init code Nicolai Stange <nicstange@gmail.com> - 2017-01-13 00:20 +0100
        Re: [PATCH 2/4] efi/x86: move efi bgrt init code to early init code Dave Young <dyoung@redhat.com> - 2017-01-13 03:30 +0100
          Re: [PATCH 2/4] efi/x86: move efi bgrt init code to early init code Dave Young <dyoung@redhat.com> - 2017-01-13 04:10 +0100
            Re: [PATCH 2/4] efi/x86: move efi bgrt init code to early init code Nicolai Stange <nicstange@gmail.com> - 2017-01-13 13:30 +0100
              Re: [PATCH 2/4] efi/x86: move efi bgrt init code to early init code Dave Young <dyoung@redhat.com> - 2017-01-16 04:00 +0100
  Re: [PATCH 2/4] efi/x86: move efi bgrt init code to early init code Ard Biesheuvel <ard.biesheuvel@linaro.org> - 2017-01-12 17:30 +0100
    Re: [PATCH 2/4] efi/x86: move efi bgrt init code to early init code Dave Young <dyoung@redhat.com> - 2017-01-12 22:40 +0100
      Re: [PATCH 2/4] efi/x86: move efi bgrt init code to early init code Bhupesh Sharma <bhsharma@redhat.com> - 2017-01-16 16:20 +0100

csiph-web