* [PATCH] iio: light: opt3001: fix mutex unlock race
@ 2019-09-19 22:54 David Frey
2019-09-20 17:40 ` Andreas Dannenberg
0 siblings, 1 reply; 3+ messages in thread
From: David Frey @ 2019-09-19 22:54 UTC (permalink / raw)
To: linux-iio; +Cc: dannenberg, David Frey
When an end-of-conversion interrupt is received after performing a
single-shot reading of the light sensor, the driver was waking up the
result ready queue before checking opt->ok_to_ignore_lock to determine
if it should unlock the mutex. The problem occurred in the case where
the other thread woke up and changed the value of opt->ok_to_ignore_lock
to false prior to the interrupt thread performing its read of the
variable. In this case, the mutex would be unlocked twice.
Signed-off-by: David Frey <dpfrey@gmail.com>
---
drivers/iio/light/opt3001.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/iio/light/opt3001.c b/drivers/iio/light/opt3001.c
index e666879007d2..92004a2563ea 100644
--- a/drivers/iio/light/opt3001.c
+++ b/drivers/iio/light/opt3001.c
@@ -686,6 +686,7 @@ static irqreturn_t opt3001_irq(int irq, void *_iio)
struct iio_dev *iio = _iio;
struct opt3001 *opt = iio_priv(iio);
int ret;
+ bool wake_result_ready_queue = false;
if (!opt->ok_to_ignore_lock)
mutex_lock(&opt->lock);
@@ -720,13 +721,16 @@ static irqreturn_t opt3001_irq(int irq, void *_iio)
}
opt->result = ret;
opt->result_ready = true;
- wake_up(&opt->result_ready_queue);
+ wake_result_ready_queue = true;
}
out:
if (!opt->ok_to_ignore_lock)
mutex_unlock(&opt->lock);
+ if (wake_result_ready_queue)
+ wake_up(&opt->result_ready_queue);
+
return IRQ_HANDLED;
}
--
2.23.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] iio: light: opt3001: fix mutex unlock race
2019-09-19 22:54 [PATCH] iio: light: opt3001: fix mutex unlock race David Frey
@ 2019-09-20 17:40 ` Andreas Dannenberg
2019-10-05 15:09 ` Jonathan Cameron
0 siblings, 1 reply; 3+ messages in thread
From: Andreas Dannenberg @ 2019-09-20 17:40 UTC (permalink / raw)
To: David Frey; +Cc: linux-iio
David,
On Thu, Sep 19, 2019 at 03:54:18PM -0700, David Frey wrote:
> When an end-of-conversion interrupt is received after performing a
> single-shot reading of the light sensor, the driver was waking up the
> result ready queue before checking opt->ok_to_ignore_lock to determine
> if it should unlock the mutex. The problem occurred in the case where
> the other thread woke up and changed the value of opt->ok_to_ignore_lock
> to false prior to the interrupt thread performing its read of the
> variable. In this case, the mutex would be unlocked twice.
>
> Signed-off-by: David Frey <dpfrey@gmail.com>
> ---
Good find, thanks for the submission.
Reviewed-by: Andreas Dannenberg <dannenberg@ti.com>
--
Andreas Dannenberg
Texas Instruments Inc
> drivers/iio/light/opt3001.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/iio/light/opt3001.c b/drivers/iio/light/opt3001.c
> index e666879007d2..92004a2563ea 100644
> --- a/drivers/iio/light/opt3001.c
> +++ b/drivers/iio/light/opt3001.c
> @@ -686,6 +686,7 @@ static irqreturn_t opt3001_irq(int irq, void *_iio)
> struct iio_dev *iio = _iio;
> struct opt3001 *opt = iio_priv(iio);
> int ret;
> + bool wake_result_ready_queue = false;
>
> if (!opt->ok_to_ignore_lock)
> mutex_lock(&opt->lock);
> @@ -720,13 +721,16 @@ static irqreturn_t opt3001_irq(int irq, void *_iio)
> }
> opt->result = ret;
> opt->result_ready = true;
> - wake_up(&opt->result_ready_queue);
> + wake_result_ready_queue = true;
> }
>
> out:
> if (!opt->ok_to_ignore_lock)
> mutex_unlock(&opt->lock);
>
> + if (wake_result_ready_queue)
> + wake_up(&opt->result_ready_queue);
> +
> return IRQ_HANDLED;
> }
>
> --
> 2.23.0
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] iio: light: opt3001: fix mutex unlock race
2019-09-20 17:40 ` Andreas Dannenberg
@ 2019-10-05 15:09 ` Jonathan Cameron
0 siblings, 0 replies; 3+ messages in thread
From: Jonathan Cameron @ 2019-10-05 15:09 UTC (permalink / raw)
To: Andreas Dannenberg; +Cc: David Frey, linux-iio
On Fri, 20 Sep 2019 12:40:37 -0500
Andreas Dannenberg <dannenberg@ti.com> wrote:
> David,
>
> On Thu, Sep 19, 2019 at 03:54:18PM -0700, David Frey wrote:
> > When an end-of-conversion interrupt is received after performing a
> > single-shot reading of the light sensor, the driver was waking up the
> > result ready queue before checking opt->ok_to_ignore_lock to determine
> > if it should unlock the mutex. The problem occurred in the case where
> > the other thread woke up and changed the value of opt->ok_to_ignore_lock
> > to false prior to the interrupt thread performing its read of the
> > variable. In this case, the mutex would be unlocked twice.
> >
> > Signed-off-by: David Frey <dpfrey@gmail.com>
> > ---
>
> Good find, thanks for the submission.
>
> Reviewed-by: Andreas Dannenberg <dannenberg@ti.com>
I think this goes all the way back to the initial driver so I've added
a fixes tag for that and marked it for stable.
Applied to the fixes-togreg branch of iio.git.
Thanks,
Jonathan
>
>
> --
> Andreas Dannenberg
> Texas Instruments Inc
>
> > drivers/iio/light/opt3001.c | 6 +++++-
> > 1 file changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/iio/light/opt3001.c b/drivers/iio/light/opt3001.c
> > index e666879007d2..92004a2563ea 100644
> > --- a/drivers/iio/light/opt3001.c
> > +++ b/drivers/iio/light/opt3001.c
> > @@ -686,6 +686,7 @@ static irqreturn_t opt3001_irq(int irq, void *_iio)
> > struct iio_dev *iio = _iio;
> > struct opt3001 *opt = iio_priv(iio);
> > int ret;
> > + bool wake_result_ready_queue = false;
> >
> > if (!opt->ok_to_ignore_lock)
> > mutex_lock(&opt->lock);
> > @@ -720,13 +721,16 @@ static irqreturn_t opt3001_irq(int irq, void *_iio)
> > }
> > opt->result = ret;
> > opt->result_ready = true;
> > - wake_up(&opt->result_ready_queue);
> > + wake_result_ready_queue = true;
> > }
> >
> > out:
> > if (!opt->ok_to_ignore_lock)
> > mutex_unlock(&opt->lock);
> >
> > + if (wake_result_ready_queue)
> > + wake_up(&opt->result_ready_queue);
> > +
> > return IRQ_HANDLED;
> > }
> >
> > --
> > 2.23.0
> >
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2019-10-05 15:10 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-19 22:54 [PATCH] iio: light: opt3001: fix mutex unlock race David Frey
2019-09-20 17:40 ` Andreas Dannenberg
2019-10-05 15:09 ` Jonathan Cameron
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.