| Message ID | 20240814-check_pci_probe_res-v2-1-a03c8c9b498b@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] ACPI: PCI: check if the io space is page aligned | expand |
On Wed, 14 Aug 2024, Miao Wang via B4 Relay wrote: > From: Miao Wang <shankerwangmiao@gmail.com> > > When the IO resource given by _CRS method is not page aligned, serious Is this seen on some system? Could that be documented for future reference. Also, I suggest you add "root" into the shortlog and desciption to explain the limited scope of this change because IIRC, somebody wanted to allow <4k aligned IO resources under certain conditions. > problems will happen because the mis-aligend address is passed down to aligned > pci_remap_iospace, then to vmap_page_range, and finally to Add () to any function names. > vmap_pte_range, where the length bewteen addr and end is expected to be between > divisible by PAGE_SIZE, or the loop will overrun till the pfn_none check > fails. > > Signed-off-by: Miao Wang <shankerwangmiao@gmail.com> > --- > Changes in v2: > - Sorry for posting out the draft version in V1, fixed a silly compiling issue. > - Link to v1: https://lore.kernel.org/r/20240814-check_pci_probe_res-v1-1-122ee07821ab@gmail.com > --- > drivers/acpi/pci_root.c | 14 +++++++++++--- > 1 file changed, 11 insertions(+), 3 deletions(-) > > diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c > index d0bfb3706801..706449e5f29b 100644 > --- a/drivers/acpi/pci_root.c > +++ b/drivers/acpi/pci_root.c > @@ -858,7 +858,7 @@ static void acpi_pci_root_validate_resources(struct device *dev, > } > } > > -static void acpi_pci_root_remap_iospace(struct fwnode_handle *fwnode, > +static void acpi_pci_root_remap_iospace(struct acpi_device *device, > struct resource_entry *entry) > { > #ifdef PCI_IOBASE > @@ -868,7 +868,15 @@ static void acpi_pci_root_remap_iospace(struct fwnode_handle *fwnode, > resource_size_t length = resource_size(res); > unsigned long port; > > - if (pci_register_io_range(fwnode, cpu_addr, length)) > + if (!PAGE_ALIGNED(cpu_addr) || !PAGE_ALIGNED(length) || > + !PAGE_ALIGNED(pci_addr)) { Align this line so that ! are under each other. As is, it misleadingly looks on a quick glance to belong into the code block. > + dev_err(&device->dev, > + FW_BUG "I/O resource %pR or its offset %pa is not page aligned\n", > + res, &entry->offset); > + goto err; > + } > + > + if (pci_register_io_range(&device->fwnode, cpu_addr, length)) > goto err; > > port = pci_address_to_pio(cpu_addr); > @@ -910,7 +918,7 @@ int acpi_pci_probe_root_resources(struct acpi_pci_root_info *info) > else { > resource_list_for_each_entry_safe(entry, tmp, list) { > if (entry->res->flags & IORESOURCE_IO) > - acpi_pci_root_remap_iospace(&device->fwnode, > + acpi_pci_root_remap_iospace(device, > entry); > > if (entry->res->flags & IORESOURCE_DISABLED)
diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c index d0bfb3706801..706449e5f29b 100644 --- a/drivers/acpi/pci_root.c +++ b/drivers/acpi/pci_root.c @@ -858,7 +858,7 @@ static void acpi_pci_root_validate_resources(struct device *dev, } } -static void acpi_pci_root_remap_iospace(struct fwnode_handle *fwnode, +static void acpi_pci_root_remap_iospace(struct acpi_device *device, struct resource_entry *entry) { #ifdef PCI_IOBASE @@ -868,7 +868,15 @@ static void acpi_pci_root_remap_iospace(struct fwnode_handle *fwnode, resource_size_t length = resource_size(res); unsigned long port; - if (pci_register_io_range(fwnode, cpu_addr, length)) + if (!PAGE_ALIGNED(cpu_addr) || !PAGE_ALIGNED(length) || + !PAGE_ALIGNED(pci_addr)) { + dev_err(&device->dev, + FW_BUG "I/O resource %pR or its offset %pa is not page aligned\n", + res, &entry->offset); + goto err; + } + + if (pci_register_io_range(&device->fwnode, cpu_addr, length)) goto err; port = pci_address_to_pio(cpu_addr); @@ -910,7 +918,7 @@ int acpi_pci_probe_root_resources(struct acpi_pci_root_info *info) else { resource_list_for_each_entry_safe(entry, tmp, list) { if (entry->res->flags & IORESOURCE_IO) - acpi_pci_root_remap_iospace(&device->fwnode, + acpi_pci_root_remap_iospace(device, entry); if (entry->res->flags & IORESOURCE_DISABLED)