I apologize for the confusion.
Re-starting this thread wasn’t much about adding more reviewer names.
What I meant was what came out of his review:
diff --git a/drivers/pci/switch/switchtec.c b/drivers/pci/switch/switchtec.c
index 073d0f7f5e43..b1990bde688c 100644
--- a/drivers/pci/switch/switchtec.c
+++ b/drivers/pci/switch/switchtec.c
@@ -1387,6 +1387,7 @@ static struct switchtec_dev *stdev_create(struct pci_dev *pdev)
err_put:
put_device(&stdev->dev);
+ pci_dev_put(stdev->pdev);
return ERR_PTR(rc);
}
I probably should not’ have put that patch fragment at the far end of my email, past the signature.
If you prefer a v4, I’ll make a v4.
Sorry again,
Daniel
> On Nov 21, 2023, at 4:41 PM, Bjorn Helgaas <helgaas@xxxxxxxxxx> wrote:
>
> On Wed, Nov 22, 2023 at 12:37:33AM +0000, Dmitry Safonov wrote:
>> On 11/13/23 21:21, 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
>>> counted one. Therefore, in dma mode, the iowrite32 in stdev_release
>>> will cause a fatal page fault, and the subsequent dma_free_coherent,
>>> if reached, would pass a stale &stdev->pdev->dev pointer.
>>>
>>> Fixed by moving mrpc dma shutdown into switchtec_pci_remove, after
>>> stdev_kill. Counting the stdev->pdev ref is now optional, but may
>>> prevent future accidents.
>>>
>>> Signed-off-by: Daniel Stodden <dns@xxxxxxxxxx>
>>> Reviewed-by: Logan Gunthorpe <logang@xxxxxxxxxxxx>
>>
>> Just in case, duplicating on the patch.
>> With pci_dev_put(stdev->pdev) on stdev_create() err-path,
>>
>> Reviewed-by: Dmitry Safonov <dima@xxxxxxxxxx>
>
> OK, I'm totally lost. Please post a v4 with the content you want.
>
> Bjorn
