[PATCH] spi: Fix race condition in stop

linux-spi.vger.kernel.org archive mirror
 help / color / mirror / Atom feed

* [PATCH] spi: Fix race condition in stop_queue()
       [not found] ` <201104061746.44134.anarsoul-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
@ 2011-04-06 14:49   ` Vasily Khoruzhick
  0 siblings, 0 replies; 4+ messages in thread
From: Vasily Khoruzhick @ 2011-04-06 14:49 UTC (permalink / raw)
  To: Eric Miao, Mike Frysinger, Marek Vasut,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, David Brownell
  Cc: Vasily Khoruzhick

There's a race condition in stop_queue() in some drivers -
if drv_data->queue is empty, but drv_data->busy is still set
(or opposite situation) stop_queue will return -EBUSY.
So fix loop condition to check that both drv_data->queue is empty
and drv_data->busy is not set.

This patch affects following drivers:
pxa2xx_spi
spi_bfin5xx
amba-pl022
dw_spi

Signed-off-by: Vasily Khoruzhick <anarsoul-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 drivers/spi/amba-pl022.c  |    2 +-
 drivers/spi/dw_spi.c      |    2 +-
 drivers/spi/pxa2xx_spi.c  |    2 +-
 drivers/spi/spi_bfin5xx.c |    2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/spi/amba-pl022.c b/drivers/spi/amba-pl022.c
index 71a1219..23aaaa9 100644
--- a/drivers/spi/amba-pl022.c
+++ b/drivers/spi/amba-pl022.c
@@ -1553,7 +1553,7 @@ static int stop_queue(struct pl022 *pl022)
 	 * A wait_queue on the pl022->busy could be used, but then the common
 	 * execution path (pump_messages) would be required to call wake_up or
 	 * friends on every SPI message. Do this instead */
-	while (!list_empty(&pl022->queue) && pl022->busy && limit--) {
+	while ((!list_empty(&pl022->queue) || pl022->busy) && limit--) {
 		spin_unlock_irqrestore(&pl022->queue_lock, flags);
 		msleep(10);
 		spin_lock_irqsave(&pl022->queue_lock, flags);
diff --git a/drivers/spi/dw_spi.c b/drivers/spi/dw_spi.c
index 22af77f..b53be4d 100644
--- a/drivers/spi/dw_spi.c
+++ b/drivers/spi/dw_spi.c
@@ -821,7 +821,7 @@ static int stop_queue(struct dw_spi *dws)
 
 	spin_lock_irqsave(&dws->lock, flags);
 	dws->run = QUEUE_STOPPED;
-	while (!list_empty(&dws->queue) && dws->busy && limit--) {
+	while ((!list_empty(&dws->queue) || dws->busy) && limit--) {
 		spin_unlock_irqrestore(&dws->lock, flags);
 		msleep(10);
 		spin_lock_irqsave(&dws->lock, flags);
diff --git a/drivers/spi/pxa2xx_spi.c b/drivers/spi/pxa2xx_spi.c
index 9592883..39cc10f 100644
--- a/drivers/spi/pxa2xx_spi.c
+++ b/drivers/spi/pxa2xx_spi.c
@@ -1493,7 +1493,7 @@ static int stop_queue(struct driver_data *drv_data)
 	 * execution path (pump_messages) would be required to call wake_up or
 	 * friends on every SPI message. Do this instead */
 	drv_data->run = QUEUE_STOPPED;
-	while (!list_empty(&drv_data->queue) && drv_data->busy && limit--) {
+	while ((!list_empty(&drv_data->queue) || drv_data->busy) && limit--) {
 		spin_unlock_irqrestore(&drv_data->lock, flags);
 		msleep(10);
 		spin_lock_irqsave(&drv_data->lock, flags);
diff --git a/drivers/spi/spi_bfin5xx.c b/drivers/spi/spi_bfin5xx.c
index 3f22351..0e14f40 100644
--- a/drivers/spi/spi_bfin5xx.c
+++ b/drivers/spi/spi_bfin5xx.c
@@ -1244,7 +1244,7 @@ static inline int bfin_spi_stop_queue(struct bfin_spi_master_data *drv_data)
 	 * friends on every SPI message. Do this instead
 	 */
 	drv_data->running = false;
-	while (!list_empty(&drv_data->queue) && drv_data->busy && limit--) {
+	while ((!list_empty(&drv_data->queue) || drv_data->busy) && limit--) {
 		spin_unlock_irqrestore(&drv_data->lock, flags);
 		msleep(10);
 		spin_lock_irqsave(&drv_data->lock, flags);
-- 
1.7.4.1


------------------------------------------------------------------------------
Xperia(TM) PLAY
It's a major breakthrough. An authentic gaming
smartphone on the nation's most reliable network.
And it wants your games.
http://p.sf.net/sfu/verizon-sfdev

^ permalink raw reply related	[flat|nested] 4+ messages in thread

[parent not found: <1302101355-31187-1-git-send-email-anarsoul@gmail.com>]

[parent not found: <1302101355-31187-1-git-send-email-anarsoul-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]

* Re: [PATCH] spi: Fix race condition in stop_queue()
       [not found]   ` <1302101355-31187-1-git-send-email-anarsoul-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
@ 2011-04-07  6:01     ` Eric Miao
  2011-04-07  6:12     ` Mike Frysinger
  2011-04-07 18:18     ` Grant Likely
  2 siblings, 0 replies; 4+ messages in thread
From: Eric Miao @ 2011-04-07  6:01 UTC (permalink / raw)
  To: Vasily Khoruzhick
  Cc: David Brownell, Mike Frysinger, Marek Vasut,
	spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r

On Wed, Apr 6, 2011 at 10:49 PM, Vasily Khoruzhick <anarsoul@gmail.com> wrote:
> There's a race condition in stop_queue() in some drivers -
> if drv_data->queue is empty, but drv_data->busy is still set
> (or opposite situation) stop_queue will return -EBUSY.
> So fix loop condition to check that both drv_data->queue is empty
> and drv_data->busy is not set.
>
> This patch affects following drivers:
> pxa2xx_spi
> spi_bfin5xx
> amba-pl022
> dw_spi
>
> Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com>

Acked-by: Eric Miao <eric.y.miao@gmail.com>

> ---
>  drivers/spi/amba-pl022.c  |    2 +-
>  drivers/spi/dw_spi.c      |    2 +-
>  drivers/spi/pxa2xx_spi.c  |    2 +-
>  drivers/spi/spi_bfin5xx.c |    2 +-
>  4 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/spi/amba-pl022.c b/drivers/spi/amba-pl022.c
> index 71a1219..23aaaa9 100644
> --- a/drivers/spi/amba-pl022.c
> +++ b/drivers/spi/amba-pl022.c
> @@ -1553,7 +1553,7 @@ static int stop_queue(struct pl022 *pl022)
>         * A wait_queue on the pl022->busy could be used, but then the common
>         * execution path (pump_messages) would be required to call wake_up or
>         * friends on every SPI message. Do this instead */
> -       while (!list_empty(&pl022->queue) && pl022->busy && limit--) {
> +       while ((!list_empty(&pl022->queue) || pl022->busy) && limit--) {
>                spin_unlock_irqrestore(&pl022->queue_lock, flags);
>                msleep(10);
>                spin_lock_irqsave(&pl022->queue_lock, flags);
> diff --git a/drivers/spi/dw_spi.c b/drivers/spi/dw_spi.c
> index 22af77f..b53be4d 100644
> --- a/drivers/spi/dw_spi.c
> +++ b/drivers/spi/dw_spi.c
> @@ -821,7 +821,7 @@ static int stop_queue(struct dw_spi *dws)
>
>        spin_lock_irqsave(&dws->lock, flags);
>        dws->run = QUEUE_STOPPED;
> -       while (!list_empty(&dws->queue) && dws->busy && limit--) {
> +       while ((!list_empty(&dws->queue) || dws->busy) && limit--) {
>                spin_unlock_irqrestore(&dws->lock, flags);
>                msleep(10);
>                spin_lock_irqsave(&dws->lock, flags);
> diff --git a/drivers/spi/pxa2xx_spi.c b/drivers/spi/pxa2xx_spi.c
> index 9592883..39cc10f 100644
> --- a/drivers/spi/pxa2xx_spi.c
> +++ b/drivers/spi/pxa2xx_spi.c
> @@ -1493,7 +1493,7 @@ static int stop_queue(struct driver_data *drv_data)
>         * execution path (pump_messages) would be required to call wake_up or
>         * friends on every SPI message. Do this instead */
>        drv_data->run = QUEUE_STOPPED;
> -       while (!list_empty(&drv_data->queue) && drv_data->busy && limit--) {
> +       while ((!list_empty(&drv_data->queue) || drv_data->busy) && limit--) {
>                spin_unlock_irqrestore(&drv_data->lock, flags);
>                msleep(10);
>                spin_lock_irqsave(&drv_data->lock, flags);
> diff --git a/drivers/spi/spi_bfin5xx.c b/drivers/spi/spi_bfin5xx.c
> index 3f22351..0e14f40 100644
> --- a/drivers/spi/spi_bfin5xx.c
> +++ b/drivers/spi/spi_bfin5xx.c
> @@ -1244,7 +1244,7 @@ static inline int bfin_spi_stop_queue(struct bfin_spi_master_data *drv_data)
>         * friends on every SPI message. Do this instead
>         */
>        drv_data->running = false;
> -       while (!list_empty(&drv_data->queue) && drv_data->busy && limit--) {
> +       while ((!list_empty(&drv_data->queue) || drv_data->busy) && limit--) {
>                spin_unlock_irqrestore(&drv_data->lock, flags);
>                msleep(10);
>                spin_lock_irqsave(&drv_data->lock, flags);
> --
> 1.7.4.1
>
>

------------------------------------------------------------------------------
Xperia(TM) PLAY
It's a major breakthrough. An authentic gaming
smartphone on the nation's most reliable network.
And it wants your games.
http://p.sf.net/sfu/verizon-sfdev
_______________________________________________
spi-devel-general mailing list
spi-devel-general@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/spi-devel-general

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] spi: Fix race condition in stop_queue()
       [not found]   ` <1302101355-31187-1-git-send-email-anarsoul-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
  2011-04-07  6:01     ` Eric Miao
@ 2011-04-07  6:12     ` Mike Frysinger
  2011-04-07 18:18     ` Grant Likely
  2 siblings, 0 replies; 4+ messages in thread
From: Mike Frysinger @ 2011-04-07  6:12 UTC (permalink / raw)
  To: Vasily Khoruzhick
  Cc: David Brownell, Eric Miao, Marek Vasut,
	spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r

On Wed, Apr 6, 2011 at 10:49, Vasily Khoruzhick wrote:
>  drivers/spi/spi_bfin5xx.c |    2 +-

seems to past my smoke test.
Acked-by: Mike Frysinger <vapier@gentoo.org>
-mike

------------------------------------------------------------------------------
Xperia(TM) PLAY
It's a major breakthrough. An authentic gaming
smartphone on the nation's most reliable network.
And it wants your games.
http://p.sf.net/sfu/verizon-sfdev
_______________________________________________
spi-devel-general mailing list
spi-devel-general@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/spi-devel-general

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] spi: Fix race condition in stop_queue()
       [not found]   ` <1302101355-31187-1-git-send-email-anarsoul-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
  2011-04-07  6:01     ` Eric Miao
  2011-04-07  6:12     ` Mike Frysinger
@ 2011-04-07 18:18     ` Grant Likely
  2 siblings, 0 replies; 4+ messages in thread
From: Grant Likely @ 2011-04-07 18:18 UTC (permalink / raw)
  To: Vasily Khoruzhick
  Cc: David Brownell, Mike Frysinger, Eric Miao, Marek Vasut,
	spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r

On Wed, Apr 06, 2011 at 05:49:15PM +0300, Vasily Khoruzhick wrote:
> There's a race condition in stop_queue() in some drivers -
> if drv_data->queue is empty, but drv_data->busy is still set
> (or opposite situation) stop_queue will return -EBUSY.
> So fix loop condition to check that both drv_data->queue is empty
> and drv_data->busy is not set.
> 
> This patch affects following drivers:
> pxa2xx_spi
> spi_bfin5xx
> amba-pl022
> dw_spi
> 
> Signed-off-by: Vasily Khoruzhick <anarsoul-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

Applied, thanks.

g.

> ---
>  drivers/spi/amba-pl022.c  |    2 +-
>  drivers/spi/dw_spi.c      |    2 +-
>  drivers/spi/pxa2xx_spi.c  |    2 +-
>  drivers/spi/spi_bfin5xx.c |    2 +-
>  4 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/spi/amba-pl022.c b/drivers/spi/amba-pl022.c
> index 71a1219..23aaaa9 100644
> --- a/drivers/spi/amba-pl022.c
> +++ b/drivers/spi/amba-pl022.c
> @@ -1553,7 +1553,7 @@ static int stop_queue(struct pl022 *pl022)
>  	 * A wait_queue on the pl022->busy could be used, but then the common
>  	 * execution path (pump_messages) would be required to call wake_up or
>  	 * friends on every SPI message. Do this instead */
> -	while (!list_empty(&pl022->queue) && pl022->busy && limit--) {
> +	while ((!list_empty(&pl022->queue) || pl022->busy) && limit--) {
>  		spin_unlock_irqrestore(&pl022->queue_lock, flags);
>  		msleep(10);
>  		spin_lock_irqsave(&pl022->queue_lock, flags);
> diff --git a/drivers/spi/dw_spi.c b/drivers/spi/dw_spi.c
> index 22af77f..b53be4d 100644
> --- a/drivers/spi/dw_spi.c
> +++ b/drivers/spi/dw_spi.c
> @@ -821,7 +821,7 @@ static int stop_queue(struct dw_spi *dws)
>  
>  	spin_lock_irqsave(&dws->lock, flags);
>  	dws->run = QUEUE_STOPPED;
> -	while (!list_empty(&dws->queue) && dws->busy && limit--) {
> +	while ((!list_empty(&dws->queue) || dws->busy) && limit--) {
>  		spin_unlock_irqrestore(&dws->lock, flags);
>  		msleep(10);
>  		spin_lock_irqsave(&dws->lock, flags);
> diff --git a/drivers/spi/pxa2xx_spi.c b/drivers/spi/pxa2xx_spi.c
> index 9592883..39cc10f 100644
> --- a/drivers/spi/pxa2xx_spi.c
> +++ b/drivers/spi/pxa2xx_spi.c
> @@ -1493,7 +1493,7 @@ static int stop_queue(struct driver_data *drv_data)
>  	 * execution path (pump_messages) would be required to call wake_up or
>  	 * friends on every SPI message. Do this instead */
>  	drv_data->run = QUEUE_STOPPED;
> -	while (!list_empty(&drv_data->queue) && drv_data->busy && limit--) {
> +	while ((!list_empty(&drv_data->queue) || drv_data->busy) && limit--) {
>  		spin_unlock_irqrestore(&drv_data->lock, flags);
>  		msleep(10);
>  		spin_lock_irqsave(&drv_data->lock, flags);
> diff --git a/drivers/spi/spi_bfin5xx.c b/drivers/spi/spi_bfin5xx.c
> index 3f22351..0e14f40 100644
> --- a/drivers/spi/spi_bfin5xx.c
> +++ b/drivers/spi/spi_bfin5xx.c
> @@ -1244,7 +1244,7 @@ static inline int bfin_spi_stop_queue(struct bfin_spi_master_data *drv_data)
>  	 * friends on every SPI message. Do this instead
>  	 */
>  	drv_data->running = false;
> -	while (!list_empty(&drv_data->queue) && drv_data->busy && limit--) {
> +	while ((!list_empty(&drv_data->queue) || drv_data->busy) && limit--) {
>  		spin_unlock_irqrestore(&drv_data->lock, flags);
>  		msleep(10);
>  		spin_lock_irqsave(&drv_data->lock, flags);
> -- 
> 1.7.4.1
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

------------------------------------------------------------------------------
Xperia(TM) PLAY
It's a major breakthrough. An authentic gaming
smartphone on the nation's most reliable network.
And it wants your games.
http://p.sf.net/sfu/verizon-sfdev

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2011-04-07 18:18 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <201104061746.44134.anarsoul@gmail.com>
     [not found] ` <201104061746.44134.anarsoul-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2011-04-06 14:49   ` [PATCH] spi: Fix race condition in stop_queue() Vasily Khoruzhick
     [not found] ` <1302101355-31187-1-git-send-email-anarsoul@gmail.com>
     [not found]   ` <1302101355-31187-1-git-send-email-anarsoul-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2011-04-07  6:01     ` Eric Miao
2011-04-07  6:12     ` Mike Frysinger
2011-04-07 18:18     ` Grant Likely

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).