| 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
| ||
|
Message-ID: <07205677-09f0-464b-b31c-0fb5493a1d81@redhat.com>
Date: Tue, 16 Sep 2025 08:52:33 +0200
From: Cédric Le Goater <clg@...hat.com>
To: Farhan Ali <alifm@...ux.ibm.com>, linux-s390@...r.kernel.org,
kvm@...r.kernel.org, linux-kernel@...r.kernel.org, linux-pci@...r.kernel.org
Cc: alex.williamson@...hat.com, helgaas@...nel.org, schnelle@...ux.ibm.com,
mjrosato@...ux.ibm.com
Subject: Re: [PATCH v3 03/10] PCI: Allow per function PCI slots
Hello Ali,
On 9/11/25 20:33, Farhan Ali wrote:
> On s390 systems, which use a machine level hypervisor, PCI devices are
> always accessed through a form of PCI pass-through which fundamentally
> operates on a per PCI function granularity. This is also reflected in the
> s390 PCI hotplug driver which creates hotplug slots for individual PCI
> functions. Its reset_slot() function, which is a wrapper for
> zpci_hot_reset_device(), thus also resets individual functions.
>
> Currently, the kernel's PCI_SLOT() macro assigns the same pci_slot object
> to multifunction devices. This approach worked fine on s390 systems that
> only exposed virtual functions as individual PCI domains to the operating
> system. Since commit 44510d6fa0c0 ("s390/pci: Handling multifunctions")
> s390 supports exposing the topology of multifunction PCI devices by
> grouping them in a shared PCI domain. When attempting to reset a function
> through the hotplug driver, the shared slot assignment causes the wrong
> function to be reset instead of the intended one. It also leaks memory as
> we do create a pci_slot object for the function, but don't correctly free
> it in pci_slot_release().
>
> Add a flag for struct pci_slot to allow per function PCI slots for
> functions managed through a hypervisor, which exposes individual PCI
> functions while retaining the topology.
>
> Fixes: 44510d6fa0c0 ("s390/pci: Handling multifunctions")
> Suggested-by: Niklas Schnelle <schnelle@...ux.ibm.com>
> Signed-off-by: Farhan Ali <alifm@...ux.ibm.com>
> ---
> drivers/pci/hotplug/s390_pci_hpc.c | 10 ++++++++--
> drivers/pci/pci.c | 4 +++-
> drivers/pci/slot.c | 14 +++++++++++---
> include/linux/pci.h | 1 +
> 4 files changed, 23 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/pci/hotplug/s390_pci_hpc.c b/drivers/pci/hotplug/s390_pci_hpc.c
> index d9996516f49e..8b547de464bf 100644
> --- a/drivers/pci/hotplug/s390_pci_hpc.c
> +++ b/drivers/pci/hotplug/s390_pci_hpc.c
> @@ -126,14 +126,20 @@ static const struct hotplug_slot_ops s390_hotplug_slot_ops = {
>
> int zpci_init_slot(struct zpci_dev *zdev)
> {
> + int ret;
> char name[SLOT_NAME_SIZE];
> struct zpci_bus *zbus = zdev->zbus;
>
> zdev->hotplug_slot.ops = &s390_hotplug_slot_ops;
>
> snprintf(name, SLOT_NAME_SIZE, "%08x", zdev->fid);
> - return pci_hp_register(&zdev->hotplug_slot, zbus->bus,
> - zdev->devfn, name);
> + ret = pci_hp_register(&zdev->hotplug_slot, zbus->bus,
> + zdev->devfn, name);
> + if (ret)
> + return ret;
> +
> + zdev->hotplug_slot.pci_slot->per_func_slot = 1;
> + return 0;
> }
>
> void zpci_exit_slot(struct zpci_dev *zdev)
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index 3994fa82df68..70296d3b1cfc 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -5061,7 +5061,9 @@ static int pci_reset_hotplug_slot(struct hotplug_slot *hotplug, bool probe)
>
> static int pci_dev_reset_slot_function(struct pci_dev *dev, bool probe)
> {
> - if (dev->multifunction || dev->subordinate || !dev->slot ||
> + if (dev->multifunction && !dev->slot->per_func_slot)
> + return -ENOTTY;
> + if (dev->subordinate || !dev->slot ||
> dev->dev_flags & PCI_DEV_FLAGS_NO_BUS_RESET)
> return -ENOTTY;
>
> diff --git a/drivers/pci/slot.c b/drivers/pci/slot.c
> index 50fb3eb595fe..51ee59e14393 100644
> --- a/drivers/pci/slot.c
> +++ b/drivers/pci/slot.c
> @@ -63,6 +63,14 @@ static ssize_t cur_speed_read_file(struct pci_slot *slot, char *buf)
> return bus_speed_read(slot->bus->cur_bus_speed, buf);
> }
>
> +static bool pci_dev_matches_slot(struct pci_dev *dev, struct pci_slot *slot)
> +{
> + if (slot->per_func_slot)
> + return dev->devfn == slot->number;
> +
> + return PCI_SLOT(dev->devfn) == slot->number;
> +}
> +
> static void pci_slot_release(struct kobject *kobj)
> {
> struct pci_dev *dev;
> @@ -73,7 +81,7 @@ static void pci_slot_release(struct kobject *kobj)
>
> down_read(&pci_bus_sem);
> list_for_each_entry(dev, &slot->bus->devices, bus_list)
> - if (PCI_SLOT(dev->devfn) == slot->number)
> + if (pci_dev_matches_slot(dev, slot))
> dev->slot = NULL;
> up_read(&pci_bus_sem);
>
> @@ -166,7 +174,7 @@ void pci_dev_assign_slot(struct pci_dev *dev)
>
> mutex_lock(&pci_slot_mutex);
> list_for_each_entry(slot, &dev->bus->slots, list)
> - if (PCI_SLOT(dev->devfn) == slot->number)
> + if (pci_dev_matches_slot(dev, slot))
> dev->slot = slot;
> mutex_unlock(&pci_slot_mutex);
> }
> @@ -285,7 +293,7 @@ struct pci_slot *pci_create_slot(struct pci_bus *parent, int slot_nr,
>
> down_read(&pci_bus_sem);
> list_for_each_entry(dev, &parent->devices, bus_list)
> - if (PCI_SLOT(dev->devfn) == slot_nr)
> + if (pci_dev_matches_slot(dev, slot))
> dev->slot = slot;
> up_read(&pci_bus_sem);
>
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 59876de13860..9265f32d9786 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -78,6 +78,7 @@ struct pci_slot {
> struct list_head list; /* Node in list of slots */
> struct hotplug_slot *hotplug; /* Hotplug info (move here) */
> unsigned char number; /* PCI_SLOT(pci_dev->devfn) */
> + unsigned int per_func_slot:1; /* Allow per function slot */
> struct kobject kobj;
> };
>
This change generates a kernel oops on x86_64. It can be reproduced in a VM.
C.
[ 3.073990] BUG: kernel NULL pointer dereference, address: 0000000000000021
[ 3.074976] #PF: supervisor read access in kernel mode
[ 3.074976] #PF: error_code(0x0000) - not-present page
[ 3.074976] PGD 0 P4D 0
[ 3.074976] Oops: Oops: 0000 [#1] SMP NOPTI
[ 3.074976] CPU: 18 UID: 0 PID: 1 Comm: swapper/0 Not tainted 6.17.0-rc6-clg-dirty #8 PREEMPT(voluntary)
[ 3.074976] Hardware name: Supermicro Super Server/X13SAE-F, BIOS 4.2 12/17/2024
[ 3.074976] RIP: 0010:pci_reset_bus_function+0xdf/0x160
[ 3.074976] Code: 4e 08 00 00 40 0f 85 83 00 00 00 48 8b 78 18 e8 27 9d ff ff 83 f8 e7 74 17 48 83 c4 08 5b 5d 41 5c c3 cc cc cc cc 48 8b 43 30 <f6> 40 21 01 75 b6 48 8b 53 10 48 83 7a 10 00 74 5e 48 83 7b 18 00
[ 3.074976] RSP: 0000:ffffcd808007b9a8 EFLAGS: 00010202
[ 3.074976] RAX: 0000000000000000 RBX: ffff88c4019b8000 RCX: 0000000000000000
[ 3.074976] RDX: 0000000000000000 RSI: 0000000000000001 RDI: ffff88c4019b8000
[ 3.074976] RBP: 0000000000000001 R08: 0000000000000002 R09: ffffcd808007b99c
[ 3.074976] R10: ffffcd808007b950 R11: 0000000000000000 R12: 0000000000000001
[ 3.074976] R13: ffff88c4019b80c8 R14: ffff88c401a7e028 R15: ffff88c401a73400
[ 3.074976] FS: 0000000000000000(0000) GS:ffff88d38aad5000(0000) knlGS:0000000000000000
[ 3.074976] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 3.074976] CR2: 0000000000000021 CR3: 0000000f66222001 CR4: 0000000000770ef0
[ 3.074976] PKRU: 55555554
[ 3.074976] Call Trace:
[ 3.074976] <TASK>
[ 3.074976] ? pci_pm_reset+0x39/0x180
[ 3.074976] pci_init_reset_methods+0x52/0x80
[ 3.074976] pci_device_add+0x215/0x5d0
[ 3.074976] pci_scan_single_device+0xa2/0xe0
[ 3.074976] pci_scan_slot+0x66/0x1c0
[ 3.074976] ? klist_next+0x145/0x150
[ 3.074976] pci_scan_child_bus_extend+0x3a/0x290
[ 3.074976] acpi_pci_root_create+0x236/0x2a0
[ 3.074976] pci_acpi_scan_root+0x19b/0x1f0
[ 3.074976] acpi_pci_root_add+0x1a5/0x370
[ 3.074976] acpi_bus_attach+0x1a8/0x290
[ 3.074976] ? __pfx_acpi_dev_for_one_check+0x10/0x10
[ 3.074976] device_for_each_child+0x4b/0x80
[ 3.074976] acpi_dev_for_each_child+0x28/0x40
[ 3.074976] ? __pfx_acpi_bus_attach+0x10/0x10
[ 3.074976] acpi_bus_attach+0x7a/0x290
[ 3.074976] ? _raw_spin_unlock_irqrestore+0x23/0x40
[ 3.074976] ? __pfx_acpi_dev_for_one_check+0x10/0x10
[ 3.074976] device_for_each_child+0x4b/0x80
[ 3.074976] acpi_dev_for_each_child+0x28/0x40
[ 3.074976] ? __pfx_acpi_bus_attach+0x10/0x10
[ 3.074976] acpi_bus_attach+0x7a/0x290
[ 3.074976] acpi_bus_scan+0x6a/0x1c0
[ 3.074976] ? __pfx_acpi_init+0x10/0x10
[ 3.074976] acpi_scan_init+0xdc/0x280
[ 3.074976] ? __pfx_acpi_init+0x10/0x10
[ 3.074976] acpi_init+0x218/0x530
[ 3.074976] do_one_initcall+0x40/0x310
[ 3.074976] kernel_init_freeable+0x2fe/0x450
[ 3.074976] ? __pfx_kernel_init+0x10/0x10
[ 3.074976] kernel_init+0x16/0x1d0
[ 3.074976] ret_from_fork+0x1ab/0x1e0
[ 3.074976] ? __pfx_kernel_init+0x10/0x10
[ 3.074976] ret_from_fork_asm+0x1a/0x30
[ 3.074976] </TASK>
[ 3.074976] Modules linked in:
[ 3.074976] CR2: 0000000000000021
[ 3.074976] ---[ end trace 0000000000000000 ]---
[ 3.074976] RIP: 0010:pci_reset_bus_function+0xdf/0x160
Powered by blists - more mailing lists