Re: [PATCH v3] switchtec: Fix stdev_release crash after suprise device loss.

From: Daniel Stodden <dns@arista.com>
To: Bjorn Helgaas <helgaas@kernel.org>, Dmitry Safonov <dima@arista.com>
Cc: Kurt Schwemmer <kurt.schwemmer@microsemi.com>,
	Logan Gunthorpe <logang@deltatee.com>,
	linux-pci@vger.kernel.org
Subject: Re: [PATCH v3] switchtec: Fix stdev_release crash after suprise device loss.
Date: Tue, 21 Nov 2023 15:58:22 -0800	[thread overview]
Message-ID: <D34BC819-ACC4-4709-8464-73EEDDC64328@arista.com> (raw)
In-Reply-To: <20231121184008.GA249064@bhelgaas>



> On Nov 21, 2023, at 10:40 AM, Bjorn Helgaas <helgaas@kernel.org> wrote:
> 
> On Tue, Nov 21, 2023 at 10:23:51AM -0800, Daniel Stodden wrote:
>>> On Nov 20, 2023, at 1:25 PM, Bjorn Helgaas <helgaas@kernel.org> wrote:
>>> On Mon, Nov 13, 2023 at 01:21:50PM -0800, Daniel Stodden wrote:
>>>> A pci device hot removal may occur while stdev->cdev is held open. The
>>>> call to stdev_release is then delivered during close or exit, at a
>>>> point way past switchtec_pci_remove. Otherwise the last ref would
>>>> vanish with the trailing put_device, just before return.
>>>> 
>>>> At that later point in time, the device layer has alreay removed
>>>> stdev->mrpc_mmio map. Also, the stdev->pdev reference was not a
>>> 
>>> I guess this should say the "stdev->mmio_mrpc" (not "mrpc_mmio")?
>> 
>> Eww. My fault. Could you still correct that?
> 
> Yep, I speculatively made that change already, so thanks for
> confirming it :)
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git/commit/?h=switchtec&id=f9724598e29d

Thanks. And sorry for what’s next. 

Look what I just found in my internal review inbox.

Signed-off/Reviewed-by: dima@arista.com

Want a v4, a follow-up, or can you re-edit that once more too, please?

Thanks + sorry,
Daniel

diff --git a/drivers/pci/switch/switchtec.c b/drivers/pci/switch/switchtec.c
index a82576205..ddaf87e10 100644
--- a/drivers/pci/switch/switchtec.c
+++ b/drivers/pci/switch/switchtec.c
@@ -1331,6 +1331,7 @@ static struct switchtec_dev *stdev_create(struct pci_dev *pdev)

 err_put:
        put_device(&stdev->dev);
+       put_device(&stdev->pdev);
        return ERR_PTR(rc);
 }



