| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Tue, 22 Mar 2016 08:02:19 -0500
From: Bjorn Helgaas <helgaas@...nel.org>
To: "Rafael J. Wysocki" <rafael@...nel.org>
Cc: Lorenzo Pieralisi <lorenzo.pieralisi@....com>,
Bjorn Helgaas <bhelgaas@...gle.com>,
Linux PCI <linux-pci@...r.kernel.org>,
ACPI Devel Maling List <linux-acpi@...r.kernel.org>,
"linux-ia64@...r.kernel.org" <linux-ia64@...r.kernel.org>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
Hanjun Guo <hanjun.guo@...aro.org>,
Jiang Liu <jiang.liu@...ux.intel.com>,
Tony Luck <tony.luck@...el.com>,
Tomasz Nowicki <tn@...ihalf.com>,
Mark Salter <msalter@...hat.com>,
"Rafael J. Wysocki" <rjw@...ysocki.net>
Subject: Re: [PATCH v3] PCI: ACPI: IA64: fix IO port generic range check
On Mon, Mar 21, 2016 at 01:42:01PM +0100, Rafael J. Wysocki wrote:
> On Mon, Mar 21, 2016 at 12:12 PM, Lorenzo Pieralisi
> <lorenzo.pieralisi@....com> wrote:
> > The [0 - 64k] ACPI PCI IO port resource boundary check in:
> >
> > acpi_dev_ioresource_flags()
> >
> > is currently applied blindly in the ACPI resource parsing to all
> > architectures, but only x86 suffers from that IO space limitation.
> >
> > On arches (ie IA64 and ARM64) where IO space is memory mapped,
> > the PCI root bridges IO resource windows are firstly initialized from
> > the _CRS (in acpi_decode_space()) and contain the CPU physical address
> > at which a root bridge decodes IO space in the CPU physical address
> > space with the offset value representing the offset required to translate
> > the PCI bus address into the CPU physical address.
> >
> > The IO resource windows are then parsed and updated in arch code
> > before creating and enumerating PCI buses (eg IA64 add_io_space())
> > to map in an arch specific way the obtained CPU physical address range
> > to a slice of virtual address space reserved to map PCI IO space,
> > ending up with PCI bridges resource windows containing IO
> > resources like the following on a working IA64 configuration:
> >
> > PCI host bridge to bus 0000:00
> > pci_bus 0000:00: root bus resource [io 0x1000000-0x100ffff window] (bus
> > address [0x0000-0xffff])
> > pci_bus 0000:00: root bus resource [mem 0x000a0000-0x000fffff window]
> > pci_bus 0000:00: root bus resource [mem 0x80000000-0x8fffffff window]
> > pci_bus 0000:00: root bus resource [mem 0x80004000000-0x800ffffffff window]
> > pci_bus 0000:00: root bus resource [bus 00]
> >
> > This implies that the [0 - 64K] check in acpi_dev_ioresource_flags()
> > leaves platforms with memory mapped IO space (ie IA64) broken (ie kernel
> > can't claim IO resources since the host bridge IO resource is disabled
> > and discarded by ACPI core code, see log on IA64 with missing root bridge
> > IO resource, silently filtered by current [0 - 64k] check in
> > acpi_dev_ioresource_flags()):
> >
> > PCI host bridge to bus 0000:00
> > pci_bus 0000:00: root bus resource [mem 0x000a0000-0x000fffff window]
> > pci_bus 0000:00: root bus resource [mem 0x80000000-0x8fffffff window]
> > pci_bus 0000:00: root bus resource [mem 0x80004000000-0x800ffffffff window]
> > pci_bus 0000:00: root bus resource [bus 00]
> >
> > [...]
> >
> > pci 0000:00:03.0: [1002:515e] type 00 class 0x030000
> > pci 0000:00:03.0: reg 0x10: [mem 0x80000000-0x87ffffff pref]
> > pci 0000:00:03.0: reg 0x14: [io 0x1000-0x10ff]
> > pci 0000:00:03.0: reg 0x18: [mem 0x88020000-0x8802ffff]
> > pci 0000:00:03.0: reg 0x30: [mem 0x88000000-0x8801ffff pref]
> > pci 0000:00:03.0: supports D1 D2
> > pci 0000:00:03.0: can't claim BAR 1 [io 0x1000-0x10ff]: no compatible
> > bridge window
> >
> > For this reason, the IO port resources boundaries check in generic ACPI
> > parsing code should be guarded with a CONFIG_X86 guard so that more arches
> > (ie ARM64) can benefit from the generic ACPI resources parsing interface
> > without incurring in unexpected resource filtering, fixing at the same
> > time current breakage on IA64.
> >
> > This patch factors out IO ports boundary [0 - 64k] check in generic ACPI
> > code and makes the IO space check X86 specific to make sure that IO
> > space resources are usable on other arches too.
> >
> > Fixes: 3772aea7d6f3 ("ia64/PCI/ACPI: Use common ACPI resource parsing
> > interface for host bridge")
> > Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@....com>
> > Cc: Bjorn Helgaas <bhelgaas@...gle.com>
> > Cc: Hanjun Guo <hanjun.guo@...aro.org>
> > Cc: Jiang Liu <jiang.liu@...ux.intel.com>
> > Cc: Tony Luck <tony.luck@...el.com>
> > Cc: Tomasz Nowicki <tn@...ihalf.com>
> > Cc: Mark Salter <msalter@...hat.com>
> > Cc: "Rafael J. Wysocki" <rjw@...ysocki.net>
> > ---
> > v2 -> v3
> >
> > - Moved IO resource check to generic ACPI resource code
> > - Dropped Tested-by tags
> > - Rebased against v4.5
> >
> > v2: https://marc.info/?l=linux-acpi&m=145521271330332&w=2
> >
> > v1 -> v2
> >
> > - Updated commit log to report missing IO resources
> > - Fixed function ioport_valid() comment 16k/64k typo
> >
> > v1: https://marc.info/?l=linux-acpi&m=145432228025354&w=2
> >
> > drivers/acpi/resource.c | 14 +++++++++++++-
> > 1 file changed, 13 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/acpi/resource.c b/drivers/acpi/resource.c
> > index d02fd53..56241eb 100644
> > --- a/drivers/acpi/resource.c
> > +++ b/drivers/acpi/resource.c
> > @@ -27,8 +27,20 @@
> >
> > #ifdef CONFIG_X86
> > #define valid_IRQ(i) (((i) != 0) && ((i) != 2))
> > +static inline bool acpi_iospace_resource_valid(struct resource *res)
> > +{
> > + /* On X86 IO space is limited to the [0 - 64K] IO port range */
> > + return res->end < 0x10003;
> > +}
> > #else
> > #define valid_IRQ(i) (true)
> > +/*
> > + * ACPI IO descriptors on arches other than X86 contain MMIO CPU physical
> > + * addresses mapping IO space in CPU physical address space, IO space
> > + * resources can be placed anywhere in the 64-bit physical address space.
> > + */
> > +static inline bool
> > +acpi_iospace_resource_valid(struct resource *res) { return true; }
> > #endif
> >
> > static bool acpi_dev_resource_len_valid(u64 start, u64 end, u64 len, bool io)
> > @@ -127,7 +139,7 @@ static void acpi_dev_ioresource_flags(struct resource *res, u64 len,
> > if (!acpi_dev_resource_len_valid(res->start, res->end, len, true))
> > res->flags |= IORESOURCE_DISABLED | IORESOURCE_UNSET;
> >
> > - if (res->end >= 0x10003)
> > + if (!acpi_iospace_resource_valid(res))
> > res->flags |= IORESOURCE_DISABLED | IORESOURCE_UNSET;
> >
> > if (io_decode == ACPI_DECODE_16)
> > --
>
> This is fine by me.
>
> Bjorn?
Looks good to me.
Bjorn
Powered by blists - more mailing lists