[PATCH] spi: Fix modular master driver remove and device suspend/remove

[PATCH] spi: Fix modular master driver remove and device suspend/remove

[stephen]

From: Stephen Street <stephen@streetfiresound.com>

Fix two problems in the spi subsystem:

1) spi subsystem core dumps when modular spi master is unloaded.
2) spi subsystem core dumps when spi slave device is suspended/resumed and
   module slave driver is not loaded.

Signed-off-by: Stephen Street <stephen@streetfiresound.com>
Signed-off-by: David Brownell <dbrownell@users.sourceforge.net>
---

 spi.c |    5 ++---
 1 files changed, 2 insertions(+), 3 deletions(-)

--- linux-2.6.16-rc2/drivers/spi/spi.c	2006-02-06 18:39:31.746537258 -0800
+++ linux-spi/drivers/spi/spi.c	2006-02-06 18:39:45.353334421 -0800
@@ -90,7 +90,7 @@ static int spi_suspend(struct device *de
 	int			value;
 	struct spi_driver	*drv = to_spi_driver(dev->driver);
 
-	if (!drv->suspend)
+	if (!drv || !drv->suspend)
 		return 0;
 
 	/* suspend will stop irqs and dma; no more i/o */
@@ -105,7 +105,7 @@ static int spi_resume(struct device *dev
 	int			value;
 	struct spi_driver	*drv = to_spi_driver(dev->driver);
 
-	if (!drv->resume)
+	if (!drv || !drv->resume)
 		return 0;
 
 	/* resume may restart the i/o queue */
@@ -449,7 +449,6 @@ void spi_unregister_master(struct spi_ma
 {
 	(void) device_for_each_child(master->cdev.dev, NULL, __unregister);
 	class_device_unregister(&master->cdev);
-	master->cdev.dev = NULL;
 }
 EXPORT_SYMBOL_GPL(spi_unregister_master);
 

Re: [PATCH] spi: Fix modular master driver remove and device suspend/remove

[Russell King]

On Mon, Feb 06, 2006 at 07:04:11PM -0800, stephen@streetfiresound.com wrote:
> --- linux-2.6.16-rc2/drivers/spi/spi.c	2006-02-06 18:39:31.746537258 -0800
> +++ linux-spi/drivers/spi/spi.c	2006-02-06 18:39:45.353334421 -0800
> @@ -90,7 +90,7 @@ static int spi_suspend(struct device *de
>  	int			value;
>  	struct spi_driver	*drv = to_spi_driver(dev->driver);
>  
> -	if (!drv->suspend)
> +	if (!drv || !drv->suspend)

Shouldn't this be dev->driver ?  If dev->driver is NULL, drv may be
non-NULL due to an offset in the structure.

> @@ -105,7 +105,7 @@ static int spi_resume(struct device *dev
>  	int			value;
>  	struct spi_driver	*drv = to_spi_driver(dev->driver);
>  
> -	if (!drv->resume)
> +	if (!drv || !drv->resume)

Ditto.

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:  2.6 Serial core

Re: [PATCH] spi: Fix modular master driver remove and device suspend/remove

[Stephen Street]

On Tue, 2006-02-07 at 08:03 +0000, Russell King wrote:
> On Mon, Feb 06, 2006 at 07:04:11PM -0800, stephen@streetfiresound.com wrote:
> > --- linux-2.6.16-rc2/drivers/spi/spi.c	2006-02-06 18:39:31.746537258 -0800
> > +++ linux-spi/drivers/spi/spi.c	2006-02-06 18:39:45.353334421 -0800
> > @@ -90,7 +90,7 @@ static int spi_suspend(struct device *de
> >  	int			value;
> >  	struct spi_driver	*drv = to_spi_driver(dev->driver);
> >  
> > -	if (!drv->suspend)
> > +	if (!drv || !drv->suspend)
> 
> Shouldn't this be dev->driver ?  If dev->driver is NULL, drv may be
> non-NULL due to an offset in the structure.
> 
If I understand your comment correctly, the implementation of to_spi_drv
protects against this by returning NULL if dev->driver is NULL. This is
implementation dependent and I can make the test explicit if you want?

-Stephen

Re: [PATCH] spi: Fix modular master driver remove and device suspend/remove

[David Brownell]

On Tuesday 07 February 2006 1:35 pm, Stephen Street wrote:
> On Tue, 2006-02-07 at 08:03 +0000, Russell King wrote:

> > > @@ -90,7 +90,7 @@ static int spi_suspend(struct device *de
> > >  	int			value;
> > >  	struct spi_driver	*drv = to_spi_driver(dev->driver);
> > >  
> > > -	if (!drv->suspend)
> > > +	if (!drv || !drv->suspend)
> > 
> > Shouldn't this be dev->driver ?  If dev->driver is NULL, drv may be
> > non-NULL due to an offset in the structure.
> > 
> If I understand your comment correctly, the implementation of to_spi_drv
> protects against this by returning NULL if dev->driver is NULL. This is
> implementation dependent 

I'd say that to_spi_driver() is defined that way, specifically
to avoid that class of nasty bug.

> and I can make the test explicit if you want? 
>