linux-kernel - Re: [PATCH v2 2/2] PCI: pciehp: Abort hot-plug if pci_hp_add

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Sun, 5 May 2024 07:45:13 +0200
From: Lukas Wunner <lukas@...ner.de>
To: Nam Cao <namcao@...utronix.de>
Cc: Bjorn Helgaas <bhelgaas@...gle.com>, Yinghai Lu <yinghai@...nel.org>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	linux-pci@...r.kernel.org, linux-kernel@...r.kernel.org,
	Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>,
	Mika Westerberg <mika.westerberg@...ux.intel.com>
Subject: Re: [PATCH v2 2/2] PCI: pciehp: Abort hot-plug if
 pci_hp_add_bridge() fails

[cc += Ilpo, Mika]

On Sat, May 04, 2024 at 06:15:22PM +0200, Nam Cao wrote:
> If a bridge is hot-added without any bus number available for its
> downstream bus, pci_hp_add_bridge() will fail. However, the driver
> proceeds regardless, and the kernel crashes.
[...]
> Fix this by aborting the hot-plug if pci_hp_add_bridge() fails.
[...]
> --- a/drivers/pci/hotplug/pciehp_pci.c
> +++ b/drivers/pci/hotplug/pciehp_pci.c
> @@ -58,8 +58,10 @@ int pciehp_configure_device(struct controller *ctrl)
>  		goto out;
>  	}
>  
> -	for_each_pci_bridge(dev, parent)
> -		pci_hp_add_bridge(dev);
> +	for_each_pci_bridge(dev, parent) {
> +		if (pci_hp_add_bridge(dev))
> +			goto out;
> +	}
>  
>  	pci_assign_unassigned_bridge_resources(bridge);
>  	pcie_bus_configure_settings(parent);

Are the curly braces even necessary?

FWIW, the rationale for returning 0 (success) in this case is that
pciehp has done its job by bringing up the slot and enumerating the
bridge in the slot.  It's not pciehp's fault that the hierarchy
cannot be extended further below the hot-added bridge.

Have you gone through the testing steps you spoke of earlier
(replacing the hot-added bridge with an Ethernet card) and do
they work correctly with this patch?

Reviewed-by: Lukas Wunner <lukas@...ner.de>

Thanks,

Lukas