lkml.org 
[lkml]   [2012]   [Jun]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    SubjectRe: [PATCH] efivars: prevent Oops if efi_enabled but no EFI runtime
    From
    On Thu, Jun 14, 2012 at 1:47 PM, Marko Kohtala <marko.kohtala@gmail.com> wrote:
    > Since v3.3-rc4-5-g1adbfa3, there has been an Oops in register_efivars
    > calling ops->get_next_variable and ending up at EIP 0 during module init.
    >
    > I boot 32-bit x86 kernel from 64-bit EFI bootloader.
    >
    > The efi_enabled is true, but runtime is not available. The functions
    > are NULL due to 32/64-bit mismatch between kernel and EFI.
    >
    > Signed-off-by: Marko Kohtala <marko.kohtala@gmail.com>
    > ---
    > I currently see this on v3.4.2.
    >
    > I could not figure out how I'm supposed to detect lack of runtime, so
    > I ended up with this quick and overly precise check that all required
    > functions are available. There may be other drivers that need to take
    > this new condition into account, so maybe Olof wants to make a better
    > fix.

    I must not have tested with efivars enabled, or I would have seen this
    when I did. Sigh.


    > diff --git a/drivers/firmware/efivars.c b/drivers/firmware/efivars.c
    > index 47408e8..612a097 100644
    > --- a/drivers/firmware/efivars.c
    > +++ b/drivers/firmware/efivars.c
    > @@ -1223,12 +1223,16 @@ efivars_init(void)
    >                return -ENOMEM;
    >        }
    >
    > -       ops.get_variable = efi.get_variable;
    > -       ops.set_variable = efi.set_variable;
    > -       ops.get_next_variable = efi.get_next_variable;
    > -       error = register_efivars(&__efivars, &ops, efi_kobj);
    > -       if (error)
    > -               goto err_put;
    > +       /* We may have efi_enabled for systab, but no runtime for variables.
    > +        * Check the functions we need before proceeding. */
    > +       if (efi.get_variable && efi.set_variable && efi.get_next_variable) {
    > +               ops.get_variable = efi.get_variable;
    > +               ops.set_variable = efi.set_variable;
    > +               ops.get_next_variable = efi.get_next_variable;
    > +               error = register_efivars(&__efivars, &ops, efi_kobj);
    > +               if (error)
    > +                       goto err_put;
    > +       }

    I think it would make more sense to return -ENODEV when the function
    pointers aren't set, that way the exit function won't ever be called
    either, so no need to add the checks there.

    So, instead of current efi_enabled check:

    if (!efi_enabled ||
    !efi.get_variable ||
    !efi.set_variable ||
    !efi.get_next_variable) {
    return -ENODEV;
    }

    -Olof
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

    \
     
     \ /
      Last update: 2012-06-18 01:41    [W:3.000 / U:0.064 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site