Linux PCI subsystem development
 help / color / mirror / Atom feed
* [PATCH] PCI/sysfs: Check IORESOURCE_DISABLED in resource mmap handler
@ 2026-05-12  8:43 Ravi Kumar Bandi
  2026-05-12  9:26 ` Krzysztof Wilczyński
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Ravi Kumar Bandi @ 2026-05-12  8:43 UTC (permalink / raw)
  To: Bjorn Helgaas; +Cc: linux-pci, linux-kernel, ravib

pci_mmap_resource() does not check IORESOURCE_DISABLED before mapping
a PCI BAR resource into userspace. This allows new mmaps to succeed
even after a device has been marked disabled or soft-unplugged by the
driver to prevent further access.

Add the check to return -ENODEV when the resource is disabled, blocking
new userspace mmaps of BAR resources after device removal.

Tested by marking the PCI BAR resource as disabled and verifying that
a subsequent mmap attempt fails with -ENODEV.

$ sudo python3 -c "
import os, mmap, errno
try:
    fd = os.open('/sys/bus/pci/devices/0001:01:00.0/resource0',os.O_RDONLY)
    mmap.mmap(fd, 4096, access=mmap.ACCESS_READ)
    print('mmap succeeded')
except OSError as e:
    print(f'mmap failed - {e}')
"
mmap failed - [Errno 19] No such device
$

Signed-off-by: Ravi Kumar Bandi <ravib@amazon.com>
---
 drivers/pci/pci-sysfs.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
index d37860841260..61ae259f8ca8 100644
--- a/drivers/pci/pci-sysfs.c
+++ b/drivers/pci/pci-sysfs.c
@@ -1089,6 +1089,9 @@ static int pci_mmap_resource(struct kobject *kobj, const struct bin_attribute *a
 	if (ret)
 		return ret;
 
+	if (res->flags & IORESOURCE_DISABLED)
+		return -ENODEV;
+
 	if (res->flags & IORESOURCE_MEM && iomem_is_exclusive(res->start))
 		return -EINVAL;
 
-- 
2.47.3


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH] PCI/sysfs: Check IORESOURCE_DISABLED in resource mmap handler
  2026-05-12  8:43 [PATCH] PCI/sysfs: Check IORESOURCE_DISABLED in resource mmap handler Ravi Kumar Bandi
@ 2026-05-12  9:26 ` Krzysztof Wilczyński
  2026-05-12  9:43   ` Ilpo Järvinen
  2026-05-13  4:13 ` [PATCH v2] PCI/sysfs: Check if device disabled " Ravi Kumar Bandi
  2026-05-13 19:42 ` [PATCH] PCI/sysfs: Check IORESOURCE_DISABLED " sashiko-bot
  2 siblings, 1 reply; 9+ messages in thread
From: Krzysztof Wilczyński @ 2026-05-12  9:26 UTC (permalink / raw)
  To: Ravi Kumar Bandi; +Cc: Bjorn Helgaas, linux-pci, linux-kernel

Hello,

> pci_mmap_resource() does not check IORESOURCE_DISABLED before mapping
> a PCI BAR resource into userspace. This allows new mmaps to succeed
> even after a device has been marked disabled or soft-unplugged by the
> driver to prevent further access.

Which driver disables resources?  Would this be some Amazon-specific thing
you are trying to fix?  Or are you just manually disabling a given device
using sysfs, or something like this?

For "soft-unplugged" device we have pci_dev_set_disconnected(), but this
does not check current flags set.

What is your use case here?

> Add the check to return -ENODEV when the resource is disabled, blocking
> new userspace mmaps of BAR resources after device removal.
> 
> Tested by marking the PCI BAR resource as disabled and verifying that
> a subsequent mmap attempt fails with -ENODEV.

Can you explain how did you do this?

> @@ -1089,6 +1089,9 @@ static int pci_mmap_resource(struct kobject *kobj, const struct bin_attribute *a
>  	if (ret)
>  		return ret;
>  
> +	if (res->flags & IORESOURCE_DISABLED)
> +		return -ENODEV;
> +

This probably would be better if it checked IORESOURCE_DISABLED and
IORESOURCE_UNSET, but then probably using resource_assigned() would
be even better.

Thank you!

	Krzysztof

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] PCI/sysfs: Check IORESOURCE_DISABLED in resource mmap handler
  2026-05-12  9:26 ` Krzysztof Wilczyński
@ 2026-05-12  9:43   ` Ilpo Järvinen
  2026-05-12  9:54     ` Krzysztof Wilczyński
  0 siblings, 1 reply; 9+ messages in thread
From: Ilpo Järvinen @ 2026-05-12  9:43 UTC (permalink / raw)
  To: Krzysztof Wilczyński, Ravi Kumar Bandi
  Cc: Bjorn Helgaas, linux-pci, LKML

[-- Attachment #1: Type: text/plain, Size: 1706 bytes --]

On Tue, 12 May 2026, Krzysztof Wilczyński wrote:

> Hello,
> 
> > pci_mmap_resource() does not check IORESOURCE_DISABLED before mapping
> > a PCI BAR resource into userspace. This allows new mmaps to succeed
> > even after a device has been marked disabled or soft-unplugged by the
> > driver to prevent further access.
> 
> Which driver disables resources?  Would this be some Amazon-specific thing
> you are trying to fix?  Or are you just manually disabling a given device
> using sysfs, or something like this?
> 
> For "soft-unplugged" device we have pci_dev_set_disconnected(), but this
> does not check current flags set.
> 
> What is your use case here?
> 
> > Add the check to return -ENODEV when the resource is disabled, blocking
> > new userspace mmaps of BAR resources after device removal.
> > 
> > Tested by marking the PCI BAR resource as disabled and verifying that
> > a subsequent mmap attempt fails with -ENODEV.
> 
> Can you explain how did you do this?
> 
> > @@ -1089,6 +1089,9 @@ static int pci_mmap_resource(struct kobject *kobj, const struct bin_attribute *a
> >  	if (ret)
> >  		return ret;
> >  
> > +	if (res->flags & IORESOURCE_DISABLED)
> > +		return -ENODEV;
> > +
> 
> This probably would be better if it checked IORESOURCE_DISABLED and
> IORESOURCE_UNSET, but then probably using resource_assigned() would
> be even better.

Yes, resource_assigned() makes more sense.

When considering Krzysztof's sysfs rework series in pci/sysfs, this all 
should be handled in .is_visible and not in pci_mmap_resource().

Also (FYI), alpha has its own pci_mmap_resource() (IIRC, it still has it 
even after Krzysztof's series).

-- 
 i.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] PCI/sysfs: Check IORESOURCE_DISABLED in resource mmap handler
  2026-05-12  9:43   ` Ilpo Järvinen
@ 2026-05-12  9:54     ` Krzysztof Wilczyński
  2026-05-13  4:04       ` Ravi Kumar Bandi
  0 siblings, 1 reply; 9+ messages in thread
From: Krzysztof Wilczyński @ 2026-05-12  9:54 UTC (permalink / raw)
  To: Ilpo Järvinen; +Cc: Ravi Kumar Bandi, Bjorn Helgaas, linux-pci, LKML

Hello,

> > > pci_mmap_resource() does not check IORESOURCE_DISABLED before mapping
> > > a PCI BAR resource into userspace. This allows new mmaps to succeed
> > > even after a device has been marked disabled or soft-unplugged by the
> > > driver to prevent further access.
> > 
> > Which driver disables resources?  Would this be some Amazon-specific thing
> > you are trying to fix?  Or are you just manually disabling a given device
> > using sysfs, or something like this?
> > 
> > For "soft-unplugged" device we have pci_dev_set_disconnected(), but this
> > does not check current flags set.
> > 
> > What is your use case here?
> > 
> > > Add the check to return -ENODEV when the resource is disabled, blocking
> > > new userspace mmaps of BAR resources after device removal.
> > > 
> > > Tested by marking the PCI BAR resource as disabled and verifying that
> > > a subsequent mmap attempt fails with -ENODEV.
> > 
> > Can you explain how did you do this?
> > 
> > > @@ -1089,6 +1089,9 @@ static int pci_mmap_resource(struct kobject *kobj, const struct bin_attribute *a
> > >  	if (ret)
> > >  		return ret;
> > >  
> > > +	if (res->flags & IORESOURCE_DISABLED)
> > > +		return -ENODEV;
> > > +
> > 
> > This probably would be better if it checked IORESOURCE_DISABLED and
> > IORESOURCE_UNSET, but then probably using resource_assigned() would
> > be even better.
> 
> Yes, resource_assigned() makes more sense.
> 
> When considering Krzysztof's sysfs rework series in pci/sysfs, this all 
> should be handled in .is_visible and not in pci_mmap_resource().

We can't use the resource_assigned() sadly, but the flags is something we
definitely can check - I will add this, test, and send new series.

The callback, though, will be called on creation, or if someone calls
remove, create or update for the sysfs resource group - no driver does this
at the moment (some DRM drivers resize resources, but that's unrelated),
so I am not sure how Ravi is doing this at runtime.  Perhaps I am looking
wrong at the code somewhere...

> Also (FYI), alpha has its own pci_mmap_resource() (IIRC, it still has it 
> even after Krzysztof's series).

For the sparse/dense resources, yes.

Thank you!

	Krzysztof

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] PCI/sysfs: Check IORESOURCE_DISABLED in resource mmap handler
  2026-05-12  9:54     ` Krzysztof Wilczyński
@ 2026-05-13  4:04       ` Ravi Kumar Bandi
  2026-05-13  5:07         ` Krzysztof Wilczyński
  0 siblings, 1 reply; 9+ messages in thread
From: Ravi Kumar Bandi @ 2026-05-13  4:04 UTC (permalink / raw)
  To: kwilczynski; +Cc: bhelgaas, ilpo.jarvinen, linux-kernel, linux-pci, ravib

On Tue, May 12, 2026 at 18:26:14 +0900, Krzysztof wrote:

> > > > pci_mmap_resource() does not check IORESOURCE_DISABLED before mapping
> > > > a PCI BAR resource into userspace. This allows new mmaps to succeed
> > > > even after a device has been marked disabled or soft-unplugged by the
> > > > driver to prevent further access.
> > >
> > > Which driver disables resources?  Would this be some Amazon-specific thing
> > > you are trying to fix?  Or are you just manually disabling a given device
> > > using sysfs, or something like this?
> > >
> > > What is your use case here?

Thank you Krzysztof, Ilpo for reviewing the patch.

The use case is surprise removal of the PCIe device that does not have a
PRSNT# routed to the CPU. Based on other internal states, the
proprietary endpoint driver marks the BAR resources as disabled by
updating this flag to prevent further userspace access to the mermory
regions (mmap) to prevent kernel hang/crash. Currently, BAR memory
regions will be allowed to be mmapped using the sysfs resource files
even if drivers soft_unplug the device.

> > > For "soft-unplugged" device we have pci_dev_set_disconnected(), but this
> > > does not check current flags set.

Thanks for the suggestion. pci_dev_set_disconnected() couldn't be used
directly as it's defined in drivers/pci/pci.h and not part of exported
kernel headers, the endpoint driver is currently setting it as below
along with the flags:

    xchg(&dev->error_state, pci_channel_io_perm_failure);
    smp_wmb();

    [...]

        pdev->resource[bar].flags |= IORESOURCE_DISABLED;

    [...]

I'll replace checking for IORESOURCE_DISABLED flag
with pci_dev_is_disconnected() instead and make endpoint driver adapt
to using pci_dev_set_disconnected().

> > > > Add the check to return -ENODEV when the resource is disabled, blocking
> > > > new userspace mmaps of BAR resources after device removal.
> > > >
> > > > Tested by marking the PCI BAR resource as disabled and verifying that
> > > > a subsequent mmap attempt fails with -ENODEV.
> > >
> > > Can you explain how did you do this?

Based on the internal logic, endpoint driver sets the resource flags as
below to indicate soft_unplug, which pci-sysfs use to to reject mmap
of the resources.

    pdev->resource[bar].flags |= IORESOURCE_DISABLED;

> > > > @@ -1089,6 +1089,9 @@ static int pci_mmap_resource(struct kobject *kobj
> > > >   if (ret)
> > > >           return ret;
> > > >
> > > > + if (res->flags & IORESOURCE_DISABLED)
> > > > +         return -ENODEV;
> > > > +
> > >
> > > This probably would be better if it checked IORESOURCE_DISABLED and
> > > IORESOURCE_UNSET, but then probably using resource_assigned() would
> > > be even better.

Thank you, but as you suggested above, I will update the patch to use
pci_dev_is_disconnected() instead.

Regards,
Ravi Kumar Bandi

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v2] PCI/sysfs: Check if device disabled in resource mmap handler
  2026-05-12  8:43 [PATCH] PCI/sysfs: Check IORESOURCE_DISABLED in resource mmap handler Ravi Kumar Bandi
  2026-05-12  9:26 ` Krzysztof Wilczyński
@ 2026-05-13  4:13 ` Ravi Kumar Bandi
  2026-05-13 19:42 ` [PATCH] PCI/sysfs: Check IORESOURCE_DISABLED " sashiko-bot
  2 siblings, 0 replies; 9+ messages in thread
From: Ravi Kumar Bandi @ 2026-05-13  4:13 UTC (permalink / raw)
  To: ravib, kwilczynski, ilpo.jarvinen; +Cc: bhelgaas, linux-kernel, linux-pci

pci_mmap_resource() does not check if device is enabled before mapping
a PCI BAR resource into userspace. This allows new mmaps to succeed
even after a device has been marked disabled or soft-unplugged by the
driver to prevent further access.

Add the check to return -ENODEV when the resource is disabled, blocking
new userspace mmaps of BAR resources after device removal.

Tested by marking the PCI BAR resource as disabled and verifying that
a subsequent mmap attempt fails with -ENODEV.

$ sudo python3 -c "
import os, mmap, errno
try:
    fd = os.open('/sys/bus/pci/devices/0001:01:00.0/resource0',os.O_RDONLY)
    mmap.mmap(fd, 4096, access=mmap.ACCESS_READ)
    print('mmap succeeded')
except OSError as e:
    print(f'mmap failed - {e}')
"
mmap failed - [Errno 19] No such device
$

Signed-off-by: Ravi Kumar Bandi <ravib@amazon.com>
---
 drivers/pci/pci-sysfs.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
index d37860841260..83d580256c4b 100644
--- a/drivers/pci/pci-sysfs.c
+++ b/drivers/pci/pci-sysfs.c
@@ -1089,6 +1089,9 @@ static int pci_mmap_resource(struct kobject *kobj, const struct bin_attribute *a
 	if (ret)
 		return ret;
 
+	if (pci_dev_is_disconnected(pdev))
+		return -ENODEV;
+
 	if (res->flags & IORESOURCE_MEM && iomem_is_exclusive(res->start))
 		return -EINVAL;
 
-- 
2.47.3


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH] PCI/sysfs: Check IORESOURCE_DISABLED in resource mmap handler
  2026-05-13  4:04       ` Ravi Kumar Bandi
@ 2026-05-13  5:07         ` Krzysztof Wilczyński
  2026-05-13  6:48           ` Ravi Kumar Bandi
  0 siblings, 1 reply; 9+ messages in thread
From: Krzysztof Wilczyński @ 2026-05-13  5:07 UTC (permalink / raw)
  To: Ravi Kumar Bandi; +Cc: bhelgaas, ilpo.jarvinen, linux-kernel, linux-pci

Hello,

> Thank you, but as you suggested above, I will update the patch to use
> pci_dev_is_disconnected() instead.

There also exists pci_device_is_present() which would attempt an actual
read of a vendor/device IDs, so if this succeeds, then you have a more
solid assurance that the device should be working.  However, this adds
latency, and there might be side-effects, potentially...

I wonder, if for you, you could forfeit setting any flags on a resource,
which drivers don't customarily do, such that we would only rely on
checking pci_dev_is_disconnected() on our side, so to speak.

We won't be setting any precedent here, as the helpers such as
pci_read_config_byte(), perform this check internally already.

Bjorn and Ilpo might have different take on this, though.

Thank you!

	Krzysztof

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] PCI/sysfs: Check IORESOURCE_DISABLED in resource mmap handler
  2026-05-13  5:07         ` Krzysztof Wilczyński
@ 2026-05-13  6:48           ` Ravi Kumar Bandi
  0 siblings, 0 replies; 9+ messages in thread
From: Ravi Kumar Bandi @ 2026-05-13  6:48 UTC (permalink / raw)
  To: kwilczynski; +Cc: bhelgaas, ilpo.jarvinen, linux-kernel, linux-pci, ravib

On Wed, May 13, 2026 at 14:07:02 +0900, Krzysztof Wilczyński wrote:
> There also exists pci_device_is_present() which would attempt an actual
> read of a vendor/device IDs, so if this succeeds, then you have a more
> solid assurance that the device should be working.  However, this adds
> latency, and there might be side-effects, potentially...

Yeah, exactly - it adds latency and will be more profound if applications
mmap the resources periodically.

> I wonder, if for you, you could forfeit setting any flags on a resource,
> which drivers don't customarily do, such that we would only rely on
> checking pci_dev_is_disconnected() on our side, so to speak.
>
> We won't be setting any precedent here, as the helpers such as
> pci_read_config_byte(), perform this check internally already.
>
> Bjorn and Ilpo might have different take on this, though.

Thank you for the suggestion. Yes, we can forfeit setting
IORESOURCE_DISABLED and rely solely on pci_dev_is_disconnected()
check in pci_mmap_resource(). I have submitted v2 patch, please
review.

Thank you.
Ravi Kumar Bandi

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] PCI/sysfs: Check IORESOURCE_DISABLED in resource mmap handler
  2026-05-12  8:43 [PATCH] PCI/sysfs: Check IORESOURCE_DISABLED in resource mmap handler Ravi Kumar Bandi
  2026-05-12  9:26 ` Krzysztof Wilczyński
  2026-05-13  4:13 ` [PATCH v2] PCI/sysfs: Check if device disabled " Ravi Kumar Bandi
@ 2026-05-13 19:42 ` sashiko-bot
  2 siblings, 0 replies; 9+ messages in thread
From: sashiko-bot @ 2026-05-13 19:42 UTC (permalink / raw)
  To: Ravi Kumar Bandi; +Cc: linux-pci

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] IORESOURCE_DISABLED is an enumeration/allocation flag, not a dynamic operational state flag.
- [High] The patch fails to check for IORESOURCE_UNSET, leaving a path to map physical address 0.
- [Medium] The check was omitted from the legacy procfs mmap interface and sysfs I/O port read/write handlers.
--

commit b337c28a9b32f2246e39d66b4dc57399f253f527
Author: Ravi Kumar Bandi <ravib@amazon.com>
PCI/sysfs: Check IORESOURCE_DISABLED in resource mmap handler

This commit attempts to block userspace from mapping PCI BAR resources of
disabled or soft-unplugged devices. It does so by adding a check for the
IORESOURCE_DISABLED flag in the sysfs pci_mmap_resource() handler.

> pci_mmap_resource() does not check IORESOURCE_DISABLED before mapping
> a PCI BAR resource into userspace. This allows new mmaps to succeed
> even after a device has been marked disabled or soft-unplugged by the
> driver to prevent further access.

Is IORESOURCE_DISABLED actually set dynamically when a device is disabled
or soft-unplugged by the driver?

Looking at the PCI core, IORESOURCE_DISABLED appears to be an allocation
flag used when a resource cannot be assigned structurally (such as a
64-bit BAR on a 32-bit system). It doesn't seem to be dynamically set
by pci_disable_device() or soft-unplug paths like
pci_stop_and_remove_bus_device().

If the flag is never set during a soft-unplug, will this check successfully
prevent the post-removal mmaps as intended?

> diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
> index d37860841260c..61ae259f8ca80 100644
> --- a/drivers/pci/pci-sysfs.c
> +++ b/drivers/pci/pci-sysfs.c
> @@ -1089,6 +1089,9 @@ static int pci_mmap_resource(struct kobject *kobj, const struct bin_attribute *a
>  	if (ret)
>  		return ret;
>  
> +	if (res->flags & IORESOURCE_DISABLED)
> +		return -ENODEV;
> +

Should this also check for IORESOURCE_UNSET?

When the PCI core fails to assign an address to a resource, it sets
IORESOURCE_UNSET and sets the resource start address to 0. Since the size
remains non-zero, the sysfs files are still created and exposed.

Without checking IORESOURCE_UNSET, could pci_mmap_resource_range() add
pci_resource_start() >> PAGE_SHIFT (which is 0) to vma->vm_pgoff, allowing
userspace to inadvertently map physical address 0 (System RAM/BIOS) instead
of an MMIO region?

Additionally, does this protection need to be extended to other interfaces?

Even with this check in sysfs mmap, userspace might still bypass it using
the legacy procfs mmap interface via proc_bus_pci_mmap().

Furthermore, for I/O port BARs (IORESOURCE_IO), the sysfs read and write
callbacks (pci_read_resource_io and pci_write_resource_io) directly execute
inb/outb instructions without verifying if the resource is disabled. Could
an application holding an open file descriptor continue raw hardware
accesses and bypass this protection?

>  	if (res->flags & IORESOURCE_MEM && iomem_is_exclusive(res->start))
>  		return -EINVAL;
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260512084315.32564-1-ravib@amazon.com?part=1

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2026-05-13 19:42 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-12  8:43 [PATCH] PCI/sysfs: Check IORESOURCE_DISABLED in resource mmap handler Ravi Kumar Bandi
2026-05-12  9:26 ` Krzysztof Wilczyński
2026-05-12  9:43   ` Ilpo Järvinen
2026-05-12  9:54     ` Krzysztof Wilczyński
2026-05-13  4:04       ` Ravi Kumar Bandi
2026-05-13  5:07         ` Krzysztof Wilczyński
2026-05-13  6:48           ` Ravi Kumar Bandi
2026-05-13  4:13 ` [PATCH v2] PCI/sysfs: Check if device disabled " Ravi Kumar Bandi
2026-05-13 19:42 ` [PATCH] PCI/sysfs: Check IORESOURCE_DISABLED " sashiko-bot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox