2006-02-07 03:04:19

by Stephen Street

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

From: Stephen Street <[email protected]>

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 <[email protected]>
Signed-off-by: David Brownell <[email protected]>
---

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);

2006-02-07 08:04:00

by Russell King

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

On Mon, Feb 06, 2006 at 07:04:11PM -0800, [email protected] 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

2006-02-07 21:35:34

by Stephen Street

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

On Tue, 2006-02-07 at 08:03 +0000, Russell King wrote:
> On Mon, Feb 06, 2006 at 07:04:11PM -0800, [email protected] 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

2006-02-07 21:56:35

by David Brownell

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

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?
>