MT7921 mt7921_pci_remove crashes

MT7921 mt7921_pci_remove crashes

[ThinerLogoer]

Sorry if this brings you the inconvenience. I am not familiar to mailing.

The issue is reported at: https://github.com/QubesOS/qubes-issues/issues/7294

`mt7921_pci_remove` seems to always crash whenever it is called. I have read the kernel source code and have a guess.

```
static void mt7921_pci_remove(struct pci_dev *pdev)
{
	struct mt76_dev *mdev = pci_get_drvdata(pdev);
	struct mt7921_dev *dev = container_of(mdev, struct mt7921_dev, mt76);

	mt7921e_unregister_device(dev);
	devm_free_irq(&pdev->dev, pdev->irq, dev);
	pci_free_irq_vectors(pdev);
}
```

From my newbie kernel knowledge I suspect that `mt7921_pci_remove` should first call `devm_free_irq` and then `mt7921e_unregister_device`, due to the reason that `devm_free_irq` calls `free_irq` that "does not return until any executing interrupts for this IRQ have completed" according to the comment there, and that when IRQ for mt7921 is being handled, it 100% uses some fields in `dev`, so before that `dev` cannot be unregistered.

This can be reproduced by echoing a pci address (for example `0000:00:07.0`) to `/sys/bus/pci/drivers/mt7921e/unbind`.

Re:MT7921 mt7921_pci_remove crashes

[ThinerLogoer]

diff '--color=auto' -u -r old/linux-5.16.13/drivers/net/wireless/mediatek/mt76/mt7921/pci.c linux-5.16.13/drivers/net/wireless/mediatek/mt76/mt7921/pci.c
--- old/linux-5.16.13/drivers/net/wireless/mediatek/mt76/mt7921/pci.c	2022-03-09 02:14:20.000000000 +0800
+++ linux-5.16.13/drivers/net/wireless/mediatek/mt76/mt7921/pci.c	2022-03-26 16:15:11.195000000 +0800
@@ -224,8 +224,8 @@
 	struct mt76_dev *mdev = pci_get_drvdata(pdev);
 	struct mt7921_dev *dev = container_of(mdev, struct mt7921_dev, mt76);
 
-	mt7921e_unregister_device(dev);
 	devm_free_irq(&pdev->dev, pdev->irq, dev);
+	mt7921e_unregister_device(dev);
 	pci_free_irq_vectors(pdev);
 }



I have hot-patched the mt7921e module with the `mt7921e_unregister_device` line and `devm_free_irq` line swapped, and now mt7921e module won't panic when it is unbinded.

Tested in 5.16.13 kernel but this should apply to most versions.

It would be of great help if you can test out this patch and merge the patch.

At 2022-03-24 14:10:05, "ThinerLogoer" <logoerthiner1@163.com> wrote:
>Sorry if this brings you the inconvenience. I am not familiar to mailing.
>
>The issue is reported at: https://github.com/QubesOS/qubes-issues/issues/7294
>
>`mt7921_pci_remove` seems to always crash whenever it is called. I have read the kernel source code and have a guess.
>
>```
>static void mt7921_pci_remove(struct pci_dev *pdev)
>{
>	struct mt76_dev *mdev = pci_get_drvdata(pdev);
>	struct mt7921_dev *dev = container_of(mdev, struct mt7921_dev, mt76);
>
>	mt7921e_unregister_device(dev);
>	devm_free_irq(&amp;pdev-&gt;dev, pdev-&gt;irq, dev);
>	pci_free_irq_vectors(pdev);
>}
>```
>
>From my newbie kernel knowledge I suspect that `mt7921_pci_remove` should first call `devm_free_irq` and then `mt7921e_unregister_device`, due to the reason that `devm_free_irq` calls `free_irq` that "does not return until any executing interrupts for this IRQ have completed" according to the comment there, and that when IRQ for mt7921 is being handled, it 100% uses some fields in `dev`, so before that `dev` cannot be unregistered.
>
>This can be reproduced by echoing a pci address (for example `0000:00:07.0`) to `/sys/bus/pci/drivers/mt7921e/unbind`.