Unverified Commit fa0561a1 authored by Mark Brown's avatar Mark Brown
Browse files

spi: Use after free fixes

Johan Hovold <johan@kernel.org> says:

The SPI subsystem frees the controller and any subsystem allocated
driver data as part of deregistration (unless the allocation is device
managed).

This series fixes the IMX driver that got this wrong and then converts
it to use device managed allocation.

Included are also a (preparatory) deregistration fix for the rockchip
driver and related cleanups for the tegre20-slink and rockchip drivers
that both take a controller reference during unbind.
parents 1fe7579a 7f3eb705
Loading
Loading
Loading
Loading
+14 −27
Original line number Diff line number Diff line
@@ -2231,11 +2231,9 @@ static int spi_imx_probe(struct platform_device *pdev)
	target_mode = devtype_data->has_targetmode &&
		      of_property_read_bool(np, "spi-slave");
	if (target_mode)
		controller = spi_alloc_target(&pdev->dev,
					      sizeof(struct spi_imx_data));
		controller = devm_spi_alloc_target(&pdev->dev, sizeof(*spi_imx));
	else
		controller = spi_alloc_host(&pdev->dev,
					    sizeof(struct spi_imx_data));
		controller = devm_spi_alloc_host(&pdev->dev, sizeof(*spi_imx));
	if (!controller)
		return -ENOMEM;

@@ -2304,40 +2302,31 @@ static int spi_imx_probe(struct platform_device *pdev)
	init_completion(&spi_imx->xfer_done);

	spi_imx->base = devm_platform_get_and_ioremap_resource(pdev, 0, &res);
	if (IS_ERR(spi_imx->base)) {
		ret = PTR_ERR(spi_imx->base);
		goto out_controller_put;
	}
	if (IS_ERR(spi_imx->base))
		return PTR_ERR(spi_imx->base);

	spi_imx->base_phys = res->start;

	irq = platform_get_irq(pdev, 0);
	if (irq < 0) {
		ret = irq;
		goto out_controller_put;
	}
	if (irq < 0)
		return irq;

	ret = devm_request_irq(&pdev->dev, irq, spi_imx_isr, 0,
			       dev_name(&pdev->dev), spi_imx);
	if (ret) {
		dev_err(&pdev->dev, "can't get irq%d: %d\n", irq, ret);
		goto out_controller_put;
	}
	if (ret)
		return dev_err_probe(&pdev->dev, ret, "can't get irq%d\n", irq);

	spi_imx->clk_ipg = devm_clk_get(&pdev->dev, "ipg");
	if (IS_ERR(spi_imx->clk_ipg)) {
		ret = PTR_ERR(spi_imx->clk_ipg);
		goto out_controller_put;
	}
	if (IS_ERR(spi_imx->clk_ipg))
		return PTR_ERR(spi_imx->clk_ipg);

	spi_imx->clk_per = devm_clk_get(&pdev->dev, "per");
	if (IS_ERR(spi_imx->clk_per)) {
		ret = PTR_ERR(spi_imx->clk_per);
		goto out_controller_put;
	}
	if (IS_ERR(spi_imx->clk_per))
		return PTR_ERR(spi_imx->clk_per);

	ret = clk_prepare_enable(spi_imx->clk_per);
	if (ret)
		goto out_controller_put;
		return ret;

	ret = clk_prepare_enable(spi_imx->clk_ipg);
	if (ret)
@@ -2389,8 +2378,6 @@ static int spi_imx_probe(struct platform_device *pdev)
	clk_disable_unprepare(spi_imx->clk_ipg);
out_put_per:
	clk_disable_unprepare(spi_imx->clk_per);
out_controller_put:
	spi_controller_put(controller);

	return ret;
}
+16 −24
Original line number Diff line number Diff line
@@ -767,9 +767,9 @@ static int rockchip_spi_probe(struct platform_device *pdev)
	target_mode = of_property_read_bool(np, "spi-slave");

	if (target_mode)
		ctlr = spi_alloc_target(&pdev->dev, sizeof(struct rockchip_spi));
		ctlr = devm_spi_alloc_target(&pdev->dev, sizeof(*rs));
	else
		ctlr = spi_alloc_host(&pdev->dev, sizeof(struct rockchip_spi));
		ctlr = devm_spi_alloc_host(&pdev->dev, sizeof(*rs));

	if (!ctlr)
		return -ENOMEM;
@@ -780,35 +780,31 @@ static int rockchip_spi_probe(struct platform_device *pdev)

	/* Get basic io resource and map it */
	rs->regs = devm_platform_get_and_ioremap_resource(pdev, 0, &mem);
	if (IS_ERR(rs->regs)) {
		ret = PTR_ERR(rs->regs);
		goto err_put_ctlr;
	}
	if (IS_ERR(rs->regs))
		return PTR_ERR(rs->regs);

	rs->apb_pclk = devm_clk_get_enabled(&pdev->dev, "apb_pclk");
	if (IS_ERR(rs->apb_pclk)) {
		ret = dev_err_probe(&pdev->dev, PTR_ERR(rs->apb_pclk),
		return dev_err_probe(&pdev->dev, PTR_ERR(rs->apb_pclk),
				     "Failed to get apb_pclk\n");
		goto err_put_ctlr;
	}

	rs->spiclk = devm_clk_get_enabled(&pdev->dev, "spiclk");
	if (IS_ERR(rs->spiclk)) {
		ret = dev_err_probe(&pdev->dev, PTR_ERR(rs->spiclk),
		return dev_err_probe(&pdev->dev, PTR_ERR(rs->spiclk),
				     "Failed to get spi_pclk\n");
		goto err_put_ctlr;
	}

	spi_enable_chip(rs, false);

	ret = platform_get_irq(pdev, 0);
	if (ret < 0)
		goto err_put_ctlr;
		return ret;

	ret = devm_request_irq(&pdev->dev, ret, rockchip_spi_isr, 0,
			       dev_name(&pdev->dev), ctlr);
	if (ret)
		goto err_put_ctlr;
		return ret;

	rs->dev = &pdev->dev;
	rs->freq = clk_get_rate(rs->spiclk);
@@ -830,10 +826,8 @@ static int rockchip_spi_probe(struct platform_device *pdev)
	}

	rs->fifo_len = get_fifo_len(rs);
	if (!rs->fifo_len) {
		ret = dev_err_probe(&pdev->dev, -EINVAL, "Failed to get fifo length\n");
		goto err_put_ctlr;
	}
	if (!rs->fifo_len)
		return dev_err_probe(&pdev->dev, -EINVAL, "Failed to get fifo length\n");

	pm_runtime_set_autosuspend_delay(&pdev->dev, ROCKCHIP_AUTOSUSPEND_TIMEOUT);
	pm_runtime_use_autosuspend(&pdev->dev);
@@ -908,7 +902,7 @@ static int rockchip_spi_probe(struct platform_device *pdev)
		break;
	}

	ret = devm_spi_register_controller(&pdev->dev, ctlr);
	ret = spi_register_controller(ctlr);
	if (ret < 0) {
		dev_err(&pdev->dev, "Failed to register controller\n");
		goto err_free_dma_rx;
@@ -924,18 +918,18 @@ static int rockchip_spi_probe(struct platform_device *pdev)
		dma_release_channel(ctlr->dma_tx);
err_disable_pm_runtime:
	pm_runtime_disable(&pdev->dev);
err_put_ctlr:
	spi_controller_put(ctlr);

	return ret;
}

static void rockchip_spi_remove(struct platform_device *pdev)
{
	struct spi_controller *ctlr = spi_controller_get(platform_get_drvdata(pdev));
	struct spi_controller *ctlr = platform_get_drvdata(pdev);

	pm_runtime_get_sync(&pdev->dev);

	spi_unregister_controller(ctlr);

	pm_runtime_put_noidle(&pdev->dev);
	pm_runtime_disable(&pdev->dev);
	pm_runtime_set_suspended(&pdev->dev);
@@ -944,8 +938,6 @@ static void rockchip_spi_remove(struct platform_device *pdev)
		dma_release_channel(ctlr->dma_tx);
	if (ctlr->dma_rx)
		dma_release_channel(ctlr->dma_rx);

	spi_controller_put(ctlr);
}

#ifdef CONFIG_PM_SLEEP
+10 −16
Original line number Diff line number Diff line
@@ -1007,7 +1007,7 @@ static int tegra_slink_probe(struct platform_device *pdev)

	cdata = of_device_get_match_data(&pdev->dev);

	host = spi_alloc_host(&pdev->dev, sizeof(*tspi));
	host = devm_spi_alloc_host(&pdev->dev, sizeof(*tspi));
	if (!host) {
		dev_err(&pdev->dev, "host allocation failed\n");
		return -ENOMEM;
@@ -1034,37 +1034,34 @@ static int tegra_slink_probe(struct platform_device *pdev)
		host->max_speed_hz = 25000000; /* 25MHz */

	tspi->base = devm_platform_get_and_ioremap_resource(pdev, 0, &r);
	if (IS_ERR(tspi->base)) {
		ret = PTR_ERR(tspi->base);
		goto exit_free_host;
	}
	if (IS_ERR(tspi->base))
		return PTR_ERR(tspi->base);

	tspi->phys = r->start;

	/* disabled clock may cause interrupt storm upon request */
	tspi->clk = devm_clk_get(&pdev->dev, NULL);
	if (IS_ERR(tspi->clk)) {
		ret = PTR_ERR(tspi->clk);
		dev_err(&pdev->dev, "Can not get clock %d\n", ret);
		goto exit_free_host;
		return dev_err_probe(&pdev->dev, ret, "Can not get clock\n");
	}

	tspi->rst = devm_reset_control_get_exclusive(&pdev->dev, "spi");
	if (IS_ERR(tspi->rst)) {
		dev_err(&pdev->dev, "can not get reset\n");
		ret = PTR_ERR(tspi->rst);
		goto exit_free_host;
		return dev_err_probe(&pdev->dev, ret, "can not get reset\n");
	}

	ret = devm_tegra_core_dev_init_opp_table_common(&pdev->dev);
	if (ret)
		goto exit_free_host;
		return ret;

	tspi->max_buf_size = SLINK_FIFO_DEPTH << 2;
	tspi->dma_buf_size = DEFAULT_SPI_DMA_BUF_LEN;

	ret = tegra_slink_init_dma_param(tspi, true);
	if (ret < 0)
		goto exit_free_host;
		return ret;
	ret = tegra_slink_init_dma_param(tspi, false);
	if (ret < 0)
		goto exit_rx_dma_free;
@@ -1125,14 +1122,13 @@ static int tegra_slink_probe(struct platform_device *pdev)
	tegra_slink_deinit_dma_param(tspi, false);
exit_rx_dma_free:
	tegra_slink_deinit_dma_param(tspi, true);
exit_free_host:
	spi_controller_put(host);

	return ret;
}

static void tegra_slink_remove(struct platform_device *pdev)
{
	struct spi_controller *host = spi_controller_get(platform_get_drvdata(pdev));
	struct spi_controller *host = platform_get_drvdata(pdev);
	struct tegra_slink_data	*tspi = spi_controller_get_devdata(host);

	spi_unregister_controller(host);
@@ -1146,8 +1142,6 @@ static void tegra_slink_remove(struct platform_device *pdev)

	if (tspi->rx_dma_chan)
		tegra_slink_deinit_dma_param(tspi, true);

	spi_controller_put(host);
}

#ifdef CONFIG_PM_SLEEP