Received-SPF: pass (google.com: domain of linux-kernel+bounces-168246-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) client-ip=2604:1380:40f1:3f00::1;
Date: Fri, 3 May 2024 16:23:41 -0500
From: Bjorn Helgaas <helgaas@kernel.org>
To: Nam Cao <namcao@linutronix.de>
Cc: Bjorn Helgaas <bhelgaas@google.com>, Yinghai Lu <yinghai@kernel.org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Rajesh Shah <rajesh.shah@intel.com>, linux-pci@vger.kernel.org,
	linux-kernel@vger.kernel.org, stable@vger.kernel.org,
	Lukas Wunner <lukas@wunner.de>
Subject: Re: [PATCH 2/4] PCI: pciehp: bail out if pci_hp_add_bridge() fails
Message-ID: <20240503212341.GA1604125@bhelgaas>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <401e4044e05d52e4243ca7faa65d5ec8b19526b8.1714762038.git.namcao@linutronix.de>

[+cc Lukas]

On Fri, May 03, 2024 at 09:23:20PM +0200, Nam Cao wrote:
> If there is no bus number available for the downstream bus of the
> hot-plugged bridge, pci_hp_add_bridge() will fail. The driver proceeds
> regardless, and the kernel crashes.
> 
> Abort if pci_hp_add_bridge() fails.

Thanks for this and for the details in the cover letter.  The cover
letter doesn't get directly preserved and connected to the git commit,
so please include some of the details here in the commit log.

I don't think we need *everything* from the cover letter; just enough
of the messages to show what went wrong and how the kernel crashed, so
somebody who trips over this can connect the crash with this fix.  And
the timestamps are not relevant, so you can strip them out.  The qemu
repro case is useful too, thanks for that!

Same for the shpchp patch.

And use "git log --oneline drivers/pci/hotplug/pciehp_pci.c" and match
the formatting (in particular, the capitalization) of your subject
lines.

> Fixes: 0eb3bcfd088e ("[PATCH] pciehp: allow bridged card hotplug")
> Signed-off-by: Nam Cao <namcao@linutronix.de>
> Cc: Rajesh Shah <rajesh.shah@intel.com>
> Cc: <stable@vger.kernel.org>
> ---
>  drivers/pci/hotplug/pciehp_pci.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/pci/hotplug/pciehp_pci.c b/drivers/pci/hotplug/pciehp_pci.c
> index ad12515a4a12..faf4fcf2fbdf 100644
> --- a/drivers/pci/hotplug/pciehp_pci.c
> +++ b/drivers/pci/hotplug/pciehp_pci.c
> @@ -58,8 +58,13 @@ 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)) {
> +			pci_stop_and_remove_bus_device(dev);
> +			ret = -EINVAL;
> +			goto out;
> +		}
> +	}
>  
>  	pci_assign_unassigned_bridge_resources(bridge);
>  	pcie_bus_configure_settings(parent);
> -- 
> 2.39.2
>