| Message ID | 7ccd9d84f7407ba65c1f03d144e2113dee0d797a.1368824585.git.luto@amacapital.net (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
On Fri, May 17, 2013 at 02:05:33PM -0700, Andy Lutomirski wrote: > The MSI MS-7760 supplies a BGRT marked "invalid" that contains a > pointer to nowhere. Since an "invalid" BGRT isn't particularly > useful (userspace isn't supposed to use it anyway), ignore the BGRT > if it's marked "invalid" and the pointer points outside of EFI boot > services space. > > Signed-off-by: Andy Lutomirski <luto@amacapital.net> I'd suggest generalizing the comment to not just mention the one system you observed it on. In any case, I'm fine with this patch, but I seem to recall Matthew Garrett having some objections to ignoring the BGRT when the valid bit is not set. Also, if you're going to do so, you might as well not expose the valid bit to userspace. > This seems to fix the problem for me. > > arch/x86/platform/efi/efi-bgrt.c | 12 ++++++++++++ > 1 file changed, 12 insertions(+) > > diff --git a/arch/x86/platform/efi/efi-bgrt.c b/arch/x86/platform/efi/efi-bgrt.c > index 7145ec6..c77b7cf 100644 > --- a/arch/x86/platform/efi/efi-bgrt.c > +++ b/arch/x86/platform/efi/efi-bgrt.c > @@ -49,6 +49,18 @@ void __init efi_bgrt_init(void) > > image = efi_lookup_mapped_addr(bgrt_tab->image_address); > if (!image) { > + if (!(bgrt_tab->status & 1)) { > + /* > + * The MSI MS-7760 exposes an "invalid" BGRT > + * containing a pointer to nowhere. This heuristic > + * will avoid following that pointer. (The idea > + * is that an "invalid" image pointing into boot > + * services data is probably sensible, but other > + * "invalid" pointers are questionable.) > + */ > + return; > + } > + > image = ioremap(bgrt_tab->image_address, sizeof(bmp_header)); > ioremapped = true; > if (!image) > -- > 1.8.1.4 > -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, May 23, 2013 at 10:37 PM, Josh Triplett <josh@joshtriplett.org> wrote: > On Fri, May 17, 2013 at 02:05:33PM -0700, Andy Lutomirski wrote: >> The MSI MS-7760 supplies a BGRT marked "invalid" that contains a >> pointer to nowhere. Since an "invalid" BGRT isn't particularly >> useful (userspace isn't supposed to use it anyway), ignore the BGRT >> if it's marked "invalid" and the pointer points outside of EFI boot >> services space. >> >> Signed-off-by: Andy Lutomirski <luto@amacapital.net> > > I'd suggest generalizing the comment to not just mention the one system > you observed it on. In any case, I'm fine with this patch, but I seem > to recall Matthew Garrett having some objections to ignoring the BGRT > when the valid bit is not set. Also, if you're going to do so, you > might as well not expose the valid bit to userspace. Hmm. Not exposing the valid bit to userspace would be a bit odd -- it's part of a bitfield which (in principle, I think) could have other bits defined. One option would be to still load the bgrt table if invalid but to not try to load the image and to therefore not show that sysfs attribute. I don't know what this would break because I don't know what userspace programs actually use bgrt. --Andy -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, May 24, 2013 at 06:15:16PM -0700, Andy Lutomirski wrote: > On Thu, May 23, 2013 at 10:37 PM, Josh Triplett <josh@joshtriplett.org> wrote: > > On Fri, May 17, 2013 at 02:05:33PM -0700, Andy Lutomirski wrote: > >> The MSI MS-7760 supplies a BGRT marked "invalid" that contains a > >> pointer to nowhere. Since an "invalid" BGRT isn't particularly > >> useful (userspace isn't supposed to use it anyway), ignore the BGRT > >> if it's marked "invalid" and the pointer points outside of EFI boot > >> services space. > >> > >> Signed-off-by: Andy Lutomirski <luto@amacapital.net> > > > > I'd suggest generalizing the comment to not just mention the one system > > you observed it on. In any case, I'm fine with this patch, but I seem > > to recall Matthew Garrett having some objections to ignoring the BGRT > > when the valid bit is not set. Also, if you're going to do so, you > > might as well not expose the valid bit to userspace. > > Hmm. > > Not exposing the valid bit to userspace would be a bit odd -- it's > part of a bitfield which (in principle, I think) could have other bits > defined. > > One option would be to still load the bgrt table if invalid but to not > try to load the image and to therefore not show that sysfs attribute. > I don't know what this would break because I don't know what userspace > programs actually use bgrt. That sounds sensible to me: there's a BGRT, so load it and expose it, but with "valid" not set, don't attempt to look at the image. - Josh Triplett -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/arch/x86/platform/efi/efi-bgrt.c b/arch/x86/platform/efi/efi-bgrt.c index 7145ec6..c77b7cf 100644 --- a/arch/x86/platform/efi/efi-bgrt.c +++ b/arch/x86/platform/efi/efi-bgrt.c @@ -49,6 +49,18 @@ void __init efi_bgrt_init(void) image = efi_lookup_mapped_addr(bgrt_tab->image_address); if (!image) { + if (!(bgrt_tab->status & 1)) { + /* + * The MSI MS-7760 exposes an "invalid" BGRT + * containing a pointer to nowhere. This heuristic + * will avoid following that pointer. (The idea + * is that an "invalid" image pointing into boot + * services data is probably sensible, but other + * "invalid" pointers are questionable.) + */ + return; + } + image = ioremap(bgrt_tab->image_address, sizeof(bmp_header)); ioremapped = true; if (!image)
The MSI MS-7760 supplies a BGRT marked "invalid" that contains a pointer to nowhere. Since an "invalid" BGRT isn't particularly useful (userspace isn't supposed to use it anyway), ignore the BGRT if it's marked "invalid" and the pointer points outside of EFI boot services space. Signed-off-by: Andy Lutomirski <luto@amacapital.net> --- This seems to fix the problem for me. arch/x86/platform/efi/efi-bgrt.c | 12 ++++++++++++ 1 file changed, 12 insertions(+)