[PATCH 1/3] spi/stmp: Fix device remove function

[PATCH 1/3] spi/stmp: Fix device remove function

[Guenter Roeck]

The call sequence spi_alloc_master/spi_register_master/spi_unregister_master
is complete; it reduces the device reference count to zero, which results in
device memory being freed. The remove function accesses the freed memory after
the call to spi_unregister_master(), _and_ it calls spi_master_put on the freed
memory.

Acquire a reference to the SPI master device and release it after cleanup is
complete (with the existing spi_master_put) to solve the problem.

Also, the device subsystem ensures that the remove function is only called once,
and resets device driver data to NULL. Remove the respective check and drop the
unnecessaary call to platform_set_drvdata().

Signed-off-by: Guenter Roeck <linux-0h96xk9xTtrk1uMJSBkQmQ@public.gmane.org>
---
Note that this driver is impossible to build, since it depends on ARCH_STMP3XXX
which is not defined anywhere.

 drivers/spi/spi-stmp.c |    6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/spi/spi-stmp.c b/drivers/spi/spi-stmp.c
index 58e3852..911e904 100644
--- a/drivers/spi/spi-stmp.c
+++ b/drivers/spi/spi-stmp.c
@@ -594,9 +594,7 @@ static int __devexit stmp_spi_remove(struct platform_device *dev)
 	struct stmp_spi *ss;
 	struct spi_master *master;
 
-	master = platform_get_drvdata(dev);
-	if (master == NULL)
-		goto out0;
+	master = spi_master_get(platform_get_drvdata(dev));
 	ss = spi_master_get_devdata(master);
 
 	spi_unregister_master(master);
@@ -609,8 +607,6 @@ static int __devexit stmp_spi_remove(struct platform_device *dev)
 	destroy_workqueue(ss->workqueue);
 	iounmap(ss->regs);
 	spi_master_put(master);
-	platform_set_drvdata(dev, NULL);
-out0:
 	return 0;
 }
 
-- 
1.7.9.7


------------------------------------------------------------------------------
Live Security Virtual Conference
Exclusive live event will cover all the ways today's security and 
threat landscape has changed and how IT managers can respond. Discussions 
will include endpoint security, mobile security and the latest in malware 
threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/

Re: [PATCH 1/3] spi/stmp: Fix device remove function

[Guenter Roeck]

On Fri, Aug 24, 2012 at 10:47:08AM +0200, Wolfram Sang wrote:
> On Thu, Aug 23, 2012 at 08:08:47PM -0700, Guenter Roeck wrote:
> > The call sequence spi_alloc_master/spi_register_master/spi_unregister_master
> > is complete; it reduces the device reference count to zero, which results in
> > device memory being freed. The remove function accesses the freed memory after
> > the call to spi_unregister_master(), _and_ it calls spi_master_put on the freed
> > memory.
> > 
> > Acquire a reference to the SPI master device and release it after cleanup is
> > complete (with the existing spi_master_put) to solve the problem.
> > 
> > Also, the device subsystem ensures that the remove function is only called once,
> > and resets device driver data to NULL. Remove the respective check and drop the
> > unnecessaary call to platform_set_drvdata().
> > 
> > Signed-off-by: Guenter Roeck <linux-0h96xk9xTtrk1uMJSBkQmQ@public.gmane.org>
> > ---
> > Note that this driver is impossible to build, since it depends on ARCH_STMP3XXX
> > which is not defined anywhere.
> 
> It can be replaced with the spi driver for mxs once it is mainline.
> 
Maybe the driver should be removed then.

The mxs driver has exactly the same problem as the one fixed with this patch.
I'll submit a patch for it.

Guenter

------------------------------------------------------------------------------
Live Security Virtual Conference
Exclusive live event will cover all the ways today's security and 
threat landscape has changed and how IT managers can respond. Discussions 
will include endpoint security, mobile security and the latest in malware 
threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/

[PATCH 2/3] spi/tegra: Fix device remove function

[Guenter Roeck]

The call to spi_unregister_master() in the device remove function frees device
memory, and with it any device local data. However, device local data is still
accessed after the call to spi_unregister_master().

Acquire a reference to the SPI master device and release it after cleanup is
complete to solve the problem.

Signed-off-by: Guenter Roeck <linux-0h96xk9xTtrk1uMJSBkQmQ@public.gmane.org>
---
 drivers/spi/spi-tegra.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/spi/spi-tegra.c b/drivers/spi/spi-tegra.c
index ef52c1c..e28445d 100644
--- a/drivers/spi/spi-tegra.c
+++ b/drivers/spi/spi-tegra.c
@@ -652,7 +652,7 @@ static int __devexit spi_tegra_remove(struct platform_device *pdev)
 	struct spi_tegra_data	*tspi;
 	struct resource		*r;
 
-	master = dev_get_drvdata(&pdev->dev);
+	master = spi_master_get(dev_get_drvdata(&pdev->dev));
 	tspi = spi_master_get_devdata(master);
 
 	spi_unregister_master(master);
@@ -668,6 +668,8 @@ static int __devexit spi_tegra_remove(struct platform_device *pdev)
 	clk_put(tspi->clk);
 	iounmap(tspi->base);
 
+	spi_master_put(master);
+
 	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	release_mem_region(r->start, resource_size(r));
 
-- 
1.7.9.7


------------------------------------------------------------------------------
Live Security Virtual Conference
Exclusive live event will cover all the ways today's security and 
threat landscape has changed and how IT managers can respond. Discussions 
will include endpoint security, mobile security and the latest in malware 
threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/

[PATCH 3/3] spi/topcliff-pch: Fix device remove function

[Guenter Roeck]

The call sequence spi_alloc_master/spi_register_master/spi_unregister_master
is complete; it reduces the device reference count to zero, which results in
device memory being freed. An extra call to spi_master_put is unnecessary
and results in an access to free memory. Drop it.

Also, the device subsystem resets device driver data to NULL after the call to
the remove function returns, so there is no need to do it here.

Signed-off-by: Guenter Roeck <linux-0h96xk9xTtrk1uMJSBkQmQ@public.gmane.org>
---
 drivers/spi/spi-topcliff-pch.c |    2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/spi/spi-topcliff-pch.c b/drivers/spi/spi-topcliff-pch.c
index cd56dcf..159dafd 100644
--- a/drivers/spi/spi-topcliff-pch.c
+++ b/drivers/spi/spi-topcliff-pch.c
@@ -1536,8 +1536,6 @@ static int __devexit pch_spi_pd_remove(struct platform_device *plat_dev)
 
 	pci_iounmap(board_dat->pdev, data->io_remap_addr);
 	spi_unregister_master(data->master);
-	spi_master_put(data->master);
-	platform_set_drvdata(plat_dev, NULL);
 
 	return 0;
 }
-- 
1.7.9.7


------------------------------------------------------------------------------
Live Security Virtual Conference
Exclusive live event will cover all the ways today's security and 
threat landscape has changed and how IT managers can respond. Discussions 
will include endpoint security, mobile security and the latest in malware 
threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/

Re: [PATCH 1/3] spi/stmp: Fix device remove function

[Mark Brown]

On Thu, Aug 23, 2012 at 08:08:47PM -0700, Guenter Roeck wrote:
> The call sequence spi_alloc_master/spi_register_master/spi_unregister_master
> is complete; it reduces the device reference count to zero, which results in
> device memory being freed. The remove function accesses the freed memory after
> the call to spi_unregister_master(), _and_ it calls spi_master_put on the freed
> memory.

Applied, thanks.

------------------------------------------------------------------------------
Live Security Virtual Conference
Exclusive live event will cover all the ways today's security and 
threat landscape has changed and how IT managers can respond. Discussions 
will include endpoint security, mobile security and the latest in malware 
threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/