lkml.org 
[lkml]   [2017]   [Jan]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    SubjectRe: [PATCH 2/4] efi/x86: move efi bgrt init code to early init code
    Date
    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.


    >> > - if (bgrt_tab->header.length < sizeof(*bgrt_tab)) {
    >> > + if (bgrt_tab.header.length < sizeof(bgrt_tab)) {
    >> > pr_notice("Ignoring BGRT: invalid length %u (expected %zu)\n",
    >> > - bgrt_tab->header.length, sizeof(*bgrt_tab));
    >> > + bgrt_tab.header.length, sizeof(bgrt_tab));
    >> > return;
    >> > }
    >>
    >> ?

    Thanks,

    Nicolai

    \
     
     \ /
      Last update: 2017-01-13 00:12    [W:3.273 / U:0.492 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site