Messages in this thread | | | Date | Sun, 17 Jun 2012 16:00:45 -0700 | Subject | Re: [PATCH] efivars: prevent Oops if efi_enabled but no EFI runtime | From | Olof Johansson <> |
| |
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/
| |