[SeaBIOS] [PATCH v2] optionrom: disallow int19 redirect for pnp roms.

Gerd Hoffmann posted 1 patch 5 years, 5 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/seabios tags/patchew/20181206150740.5609-1-kraxel@redhat.com
There is a newer version of this series
src/optionroms.c | 29 +++++++++++++++++++++++++++++
1 file changed, 29 insertions(+)
[SeaBIOS] [PATCH v2] optionrom: disallow int19 redirect for pnp roms.
Posted by Gerd Hoffmann 5 years, 5 months ago
Check whenever pnp roms attempt to redirect int19, and in case it does
log a message and undo the redirect.

A pnp rom should not need this, we have BEVs and BCVs for that.
Nevertheless there are roms in the wild which are redirecting int19.
At least some BIOS implementations for physical hardware have a config
option in the setup to allow/disallow int19 redirections, so just not
allowing this seems to be the way to deal with this situation.

Buglink: https://bugzilla.redhat.com//show_bug.cgi?id=1642135
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 src/optionroms.c | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/src/optionroms.c b/src/optionroms.c
index fc992f649f..e90900b790 100644
--- a/src/optionroms.c
+++ b/src/optionroms.c
@@ -8,6 +8,7 @@
 #include "bregs.h" // struct bregs
 #include "config.h" // CONFIG_*
 #include "farptr.h" // FLATPTR_TO_SEG
+#include "biosvar.h" // GET_IVT
 #include "hw/pci.h" // pci_config_readl
 #include "hw/pcidevice.h" // foreachpci
 #include "hw/pci_ids.h" // PCI_CLASS_DISPLAY_VGA
@@ -308,6 +309,24 @@ fail:
     return NULL;
 }
 
+static int boot_irq_captured(void)
+{
+    struct segoff_s current, seabios;
+
+    current = GET_IVT(0x19);
+    seabios = FUNC16(entry_19_official);
+    return ((current.seg != seabios.seg) ||
+            (current.offset != current.offset));
+}
+
+static void boot_irq_restore(void)
+{
+    struct segoff_s seabios;
+
+    seabios = FUNC16(entry_19_official);
+    SET_IVT(0x19, seabios);
+}
+
 // Attempt to map and initialize the option rom on a given PCI device.
 static void
 init_pcirom(struct pci_device *pci, int isvga, u64 *sources)
@@ -327,8 +346,18 @@ init_pcirom(struct pci_device *pci, int isvga, u64 *sources)
     if (! rom)
         // No ROM present.
         return;
+    int irq_was_captured = boot_irq_captured();
+    struct pnp_data *pnp = get_pnp_rom(rom);
     setRomSource(sources, rom, RS_PCIROM | (u32)pci);
     init_optionrom(rom, pci->bdf, isvga);
+    if (boot_irq_captured() && !irq_was_captured &&
+        !file && !isvga && pnp) {
+        // This PCI rom is misbehaving - recapture the boot irqs
+        char *desc = MAKE_FLATPTR(FLATPTR_TO_SEG(rom), pnp->productname);
+        dprintf(1, "PnP optionrom \"%s\" (bdf %pP) captured int19, restoring\n",
+                desc, pci);
+        boot_irq_restore();
+    }
 }
 
 
-- 
2.9.3


_______________________________________________
SeaBIOS mailing list
SeaBIOS@seabios.org
https://mail.coreboot.org/mailman/listinfo/seabios
Re: [SeaBIOS] [PATCH v2] optionrom: disallow int19 redirect for pnp roms.
Posted by Kevin O'Connor 5 years, 5 months ago
On Thu, Dec 06, 2018 at 04:07:40PM +0100, Gerd Hoffmann wrote:
> Check whenever pnp roms attempt to redirect int19, and in case it does
> log a message and undo the redirect.
> 
> A pnp rom should not need this, we have BEVs and BCVs for that.
> Nevertheless there are roms in the wild which are redirecting int19.
> At least some BIOS implementations for physical hardware have a config
> option in the setup to allow/disallow int19 redirections, so just not
> allowing this seems to be the way to deal with this situation.

Thanks.  As discussed, I'm okay with this change.

-Kevin

_______________________________________________
SeaBIOS mailing list
SeaBIOS@seabios.org
https://mail.coreboot.org/mailman/listinfo/seabios
Re: [SeaBIOS] [PATCH v2] optionrom: disallow int19 redirect for pnp roms.
Posted by Gerd Hoffmann 5 years, 5 months ago
On Mon, Dec 10, 2018 at 09:53:01PM -0500, Kevin O'Connor wrote:
> On Thu, Dec 06, 2018 at 04:07:40PM +0100, Gerd Hoffmann wrote:
> > Check whenever pnp roms attempt to redirect int19, and in case it does
> > log a message and undo the redirect.
> > 
> > A pnp rom should not need this, we have BEVs and BCVs for that.
> > Nevertheless there are roms in the wild which are redirecting int19.
> > At least some BIOS implementations for physical hardware have a config
> > option in the setup to allow/disallow int19 redirections, so just not
> > allowing this seems to be the way to deal with this situation.
> 
> Thanks.  As discussed, I'm okay with this change.

ok.  I'll wait for test results before committing.

cheers,
  Gerd


_______________________________________________
SeaBIOS mailing list
SeaBIOS@seabios.org
https://mail.coreboot.org/mailman/listinfo/seabios
[SeaBIOS] Re: [PATCH v2] optionrom: disallow int19 redirect for pnp roms.
Posted by Matt DeVillier 4 years, 11 months ago
v2 of the patch works as intended, once 'current.offset != current.offset'
is corrected to 'current.offset != seabios.offset'

cheers,
Matt

On Tue, Dec 11, 2018 at 2:03 AM Gerd Hoffmann <kraxel@redhat.com> wrote:

> On Mon, Dec 10, 2018 at 09:53:01PM -0500, Kevin O'Connor wrote:
> > On Thu, Dec 06, 2018 at 04:07:40PM +0100, Gerd Hoffmann wrote:
> > > Check whenever pnp roms attempt to redirect int19, and in case it does
> > > log a message and undo the redirect.
> > >
> > > A pnp rom should not need this, we have BEVs and BCVs for that.
> > > Nevertheless there are roms in the wild which are redirecting int19.
> > > At least some BIOS implementations for physical hardware have a config
> > > option in the setup to allow/disallow int19 redirections, so just not
> > > allowing this seems to be the way to deal with this situation.
> >
> > Thanks.  As discussed, I'm okay with this change.
>
> ok.  I'll wait for test results before committing.
>
> cheers,
>   Gerd
>
>
> _______________________________________________
> SeaBIOS mailing list
> SeaBIOS@seabios.org
> https://mail.coreboot.org/mailman/listinfo/seabios
>
_______________________________________________
SeaBIOS mailing list -- seabios@seabios.org
To unsubscribe send an email to seabios-leave@seabios.org
[SeaBIOS] Re: [PATCH v2] optionrom: disallow int19 redirect for pnp roms.
Posted by Kevin O'Connor 4 years, 11 months ago
On Mon, May 20, 2019 at 04:43:08PM -0500, Matt DeVillier wrote:
> v2 of the patch works as intended, once 'current.offset != current.offset'
> is corrected to 'current.offset != seabios.offset'

FWIW, that code could simply be:

static int boot_irq_captured(void)
{
    return GET_IVT(0x19).segoff != FUNC16(entry_19_official).segoff;
}

-Kevin
_______________________________________________
SeaBIOS mailing list -- seabios@seabios.org
To unsubscribe send an email to seabios-leave@seabios.org
[SeaBIOS] Re: [PATCH v2] optionrom: disallow int19 redirect for pnp roms.
Posted by Matt DeVillier 4 years, 11 months ago
On Mon, May 20, 2019 at 4:49 PM Kevin O'Connor <kevin@koconnor.net> wrote:

> On Mon, May 20, 2019 at 04:43:08PM -0500, Matt DeVillier wrote:
> > v2 of the patch works as intended, once 'current.offset !=
> current.offset'
> > is corrected to 'current.offset != seabios.offset'
>
> FWIW, that code could simply be:
>
> static int boot_irq_captured(void)
> {
>     return GET_IVT(0x19).segoff != FUNC16(entry_19_official).segoff;
> }
>

yes, that seems to work as well :)


>
> -Kevin
>
_______________________________________________
SeaBIOS mailing list -- seabios@seabios.org
To unsubscribe send an email to seabios-leave@seabios.org
[SeaBIOS] Re: [PATCH v2] optionrom: disallow int19 redirect for pnp roms.
Posted by Gerd Hoffmann 4 years, 11 months ago
On Mon, May 20, 2019 at 05:40:16PM -0500, Matt DeVillier wrote:
> On Mon, May 20, 2019 at 4:49 PM Kevin O'Connor <kevin@koconnor.net> wrote:
> 
> > On Mon, May 20, 2019 at 04:43:08PM -0500, Matt DeVillier wrote:
> > > v2 of the patch works as intended, once 'current.offset !=
> > current.offset'
> > > is corrected to 'current.offset != seabios.offset'
> >
> > FWIW, that code could simply be:
> >
> > static int boot_irq_captured(void)
> > {
> >     return GET_IVT(0x19).segoff != FUNC16(entry_19_official).segoff;
> > }
> >
> 
> yes, that seems to work as well :)

Thanks for testing.  Sent fixed v3.

cheers,
  Gerd
_______________________________________________
SeaBIOS mailing list -- seabios@seabios.org
To unsubscribe send an email to seabios-leave@seabios.org