On Wed, Jan 09, 2019 at 10:48:50PM +0000, Elie Roudninski wrote:
...
> [ 444.060161] Call trace:
> [ 444.062580] queued_spin_lock_slowpath+0x228/0x2d0
> [ 444.067322] meson_mmc_irq+0x290/0x2a0
> [ 444.071032] __free_irq+0x184/0x318
> [ 444.074480] free_irq+0x40/0x80
> [ 444.077586] devm_irq_release+0x24/0x30
> [ 444.081382] release_nodes+0x1e0/0x2e0
> [ 444.085089] devres_release_all+0x60/0x88
> [ 444.089057] device_release_driver_internal+0x1c8/0x248
> [ 444.094231] device_release_driver+0x28/0x38
> [ 444.098459] unbind_store+0xdc/0x148
> [ 444.101994] drv_attr_store+0x40/0x58
> [ 444.105627] sysfs_kf_write+0x5c/0x78
> [ 444.109240] kernfs_fop_write+0xe8/0x1e0
> [ 444.113126] __vfs_write+0x60/0x190
> [ 444.116570] vfs_write+0xac/0x1b0
> [ 444.119848] ksys_write+0x6c/0xd0
> [ 444.123125] __arm64_sys_write+0x24/0x30
> [ 444.127015] el0_svc_common+0x94/0xe8
> [ 444.130629] el0_svc_handler+0x38/0x80
> [ 444.134340] el0_svc+0x8/0xc
> [ 444.137187] Code: d37c0401 910020c0 8b010041 f865d8e5 (f8256826)
> [ 444.143223] ---[ end trace 62541ec2ffe020f2 ]---
> Segmentation fault
...
Here are my two cents on this.
The irq that is being freed was allocated through device management
(i.e. with devm_request_threaded_irq()). So this irq will be released
after meson_mmc_remove() completion, thus after that mmc_free_host()
has been called. Because this irq has been registered with IRQF_SHARED
and if you have CONFIG_DEBUG_SHIRQ (do you ?) the irq handler will be
called while the irq is freed.
So meson_mmc_irq could be called after mmc_free_host() has freed the
meson_host pointer causing it to dereference an invalid pointer.
So if you have CONFIG_DEBUG_SHIRQ could you please test the following
patch:
------------------------------- 8< ----------------------------------
diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
index c2690c1a50ff..3a9679531520 100644
--- a/drivers/mmc/host/meson-gx-mmc.c
+++ b/drivers/mmc/host/meson-gx-mmc.c
@@ -179,6 +179,8 @@ struct meson_host {
struct sd_emmc_desc *descs;
dma_addr_t descs_dma_addr;
+ int irq;
+
bool vqmmc_enabled;
};
@@ -1231,7 +1233,7 @@ static int meson_mmc_probe(struct platform_device *pdev)
struct resource *res;
struct meson_host *host;
struct mmc_host *mmc;
- int ret, irq;
+ int ret;
mmc = mmc_alloc_host(sizeof(struct meson_host), &pdev->dev);
if (!mmc)
@@ -1276,8 +1278,8 @@ static int meson_mmc_probe(struct platform_device *pdev)
goto free_host;
}
- irq = platform_get_irq(pdev, 0);
- if (irq <= 0) {
+ host->irq = platform_get_irq(pdev, 0);
+ if (host->irq <= 0) {
dev_err(&pdev->dev, "failed to get interrupt resource.\n");
ret = -EINVAL;
goto free_host;
@@ -1331,9 +1333,8 @@ static int meson_mmc_probe(struct platform_device *pdev)
writel(IRQ_CRC_ERR | IRQ_TIMEOUTS | IRQ_END_OF_CHAIN,
host->regs + SD_EMMC_IRQ_EN);
- ret = devm_request_threaded_irq(&pdev->dev, irq, meson_mmc_irq,
- meson_mmc_irq_thread, IRQF_SHARED,
- NULL, host);
+ ret = request_threaded_irq(host->irq, meson_mmc_irq,
+ meson_mmc_irq_thread, IRQF_SHARED, NULL, host);
if (ret)
goto err_init_clk;
@@ -1387,6 +1388,7 @@ static int meson_mmc_remove(struct platform_device *pdev)
/* disable interrupts */
writel(0, host->regs + SD_EMMC_IRQ_EN);
+ free_irq(host->irq, host);
dma_free_coherent(host->dev, SD_EMMC_DESC_BUF_LEN,
host->descs, host->descs_dma_addr);
------------------------------- 8< ----------------------------------
This basically freed the irq in the meson_mmc_remove() so the
meson_mmc_irq() is called before mmc_free_host().
--
Rémi Pommarel
