iio: imu: kmx61: Fix potential time-of-check to time-of-use race

[PATCH v2] iio: imu: kmx61: Fix potential time-of-check to time-of-use race

Posted by Maxwell Doose 1 month ago

A time-of-check to time-of-use race condition exists in
kmx61_write_event_config(). If two threads enter the function at the
same time, both threads may pass the check and get to the lock. Thus,
when the first thread releases the lock allowing the second thread to
start execution after the first thread modifies data->ev_enable_state to
force returning from the function, the second thread continues execution
regardless. Fix this by moving the data->ev_enable_state check inside of
the critical section.

Fixes: fd3ae7a9f21c ("iio: imu: kmx61: Add support for any motion trigger")
Reported-by: sashiko <sashiko-bot@kernel.org>
Closes: https://sashiko.dev/#/patchset/20260507223337.48437-1-m32285159%40gmail.com
Signed-off-by: Maxwell Doose <m32285159@gmail.com>
---
 drivers/iio/imu/kmx61.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/iio/imu/kmx61.c b/drivers/iio/imu/kmx61.c
index 3cd91d8a89ee..3afa369de3cf 100644
--- a/drivers/iio/imu/kmx61.c
+++ b/drivers/iio/imu/kmx61.c
@@ -942,11 +942,11 @@ static int kmx61_write_event_config(struct iio_dev *indio_dev,
 	struct kmx61_data *data = kmx61_get_data(indio_dev);
 	int ret = 0;
 
-	if (state && data->ev_enable_state)
-		return 0;
-
 	mutex_lock(&data->lock);
 
+	if (state && data->ev_enable_state)
+		goto err_unlock;
+
 	if (!state && data->motion_trig_on) {
 		data->ev_enable_state = false;
 		goto err_unlock;
-- 
2.54.0

Re: [PATCH v2] iio: imu: kmx61: Fix potential time-of-check to time-of-use race

Posted by Jonathan Cameron 3 weeks, 6 days ago

On Tue, 12 May 2026 20:36:38 -0500
Maxwell Doose <m32285159@gmail.com> wrote:

> A time-of-check to time-of-use race condition exists in
> kmx61_write_event_config(). If two threads enter the function at the
> same time, both threads may pass the check and get to the lock. Thus,
> when the first thread releases the lock allowing the second thread to
> start execution after the first thread modifies data->ev_enable_state to
> force returning from the function, the second thread continues execution
> regardless. Fix this by moving the data->ev_enable_state check inside of
> the critical section.
> 
> Fixes: fd3ae7a9f21c ("iio: imu: kmx61: Add support for any motion trigger")
> Reported-by: sashiko <sashiko-bot@kernel.org>
> Closes: https://sashiko.dev/#/patchset/20260507223337.48437-1-m32285159%40gmail.com
> Signed-off-by: Maxwell Doose <m32285159@gmail.com>

Sashiko has more to say.. Please take a look.  I'm rather nervous
about trying to fix the bugs it's finding around reference counting
without anyone testing.  

Maybe we can get enough confidence with emulation or stubbing.

https://sashiko.dev/#/patchset/20260513013638.147606-1-m32285159%40gmail.com

> ---
>  drivers/iio/imu/kmx61.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/iio/imu/kmx61.c b/drivers/iio/imu/kmx61.c
> index 3cd91d8a89ee..3afa369de3cf 100644
> --- a/drivers/iio/imu/kmx61.c
> +++ b/drivers/iio/imu/kmx61.c
> @@ -942,11 +942,11 @@ static int kmx61_write_event_config(struct iio_dev *indio_dev,
>  	struct kmx61_data *data = kmx61_get_data(indio_dev);
>  	int ret = 0;
>  
> -	if (state && data->ev_enable_state)
> -		return 0;
> -
>  	mutex_lock(&data->lock);
>  
> +	if (state && data->ev_enable_state)
> +		goto err_unlock;
> +
>  	if (!state && data->motion_trig_on) {
>  		data->ev_enable_state = false;
>  		goto err_unlock;

Re: [PATCH v2] iio: imu: kmx61: Fix potential time-of-check to time-of-use race

Posted by Maxwell Doose 3 weeks, 5 days ago

On Sat, May 16, 2026 at 7:55 AM Jonathan Cameron <jic23@kernel.org> wrote:
>
> On Tue, 12 May 2026 20:36:38 -0500
> Maxwell Doose <m32285159@gmail.com> wrote:
>
> > A time-of-check to time-of-use race condition exists in
> > kmx61_write_event_config(). If two threads enter the function at the
> > same time, both threads may pass the check and get to the lock. Thus,
> > when the first thread releases the lock allowing the second thread to
> > start execution after the first thread modifies data->ev_enable_state to
> > force returning from the function, the second thread continues execution
> > regardless. Fix this by moving the data->ev_enable_state check inside of
> > the critical section.
> >
> > Fixes: fd3ae7a9f21c ("iio: imu: kmx61: Add support for any motion trigger")
> > Reported-by: sashiko <sashiko-bot@kernel.org>
> > Closes: https://sashiko.dev/#/patchset/20260507223337.48437-1-m32285159%40gmail.com
> > Signed-off-by: Maxwell Doose <m32285159@gmail.com>
>
> Sashiko has more to say.. Please take a look.  I'm rather nervous
> about trying to fix the bugs it's finding around reference counting
> without anyone testing.
>
> Maybe we can get enough confidence with emulation or stubbing.
>
> https://sashiko.dev/#/patchset/20260513013638.147606-1-m32285159%40gmail.com
>

Yea, unfortunately I don't have the hardware with me. If someone else
wants to test then maybe...I haven't messed around with QEMU much so
honestly I wouldn't know how to emulate such specific hardware.

best regards,
max





> > ---
> >  drivers/iio/imu/kmx61.c | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/iio/imu/kmx61.c b/drivers/iio/imu/kmx61.c
> > index 3cd91d8a89ee..3afa369de3cf 100644
> > --- a/drivers/iio/imu/kmx61.c
> > +++ b/drivers/iio/imu/kmx61.c
> > @@ -942,11 +942,11 @@ static int kmx61_write_event_config(struct iio_dev *indio_dev,
> >       struct kmx61_data *data = kmx61_get_data(indio_dev);
> >       int ret = 0;
> >
> > -     if (state && data->ev_enable_state)
> > -             return 0;
> > -
> >       mutex_lock(&data->lock);
> >
> > +     if (state && data->ev_enable_state)
> > +             goto err_unlock;
> > +
> >       if (!state && data->motion_trig_on) {
> >               data->ev_enable_state = false;
> >               goto err_unlock;
>

Re: [PATCH v2] iio: imu: kmx61: Fix potential time-of-check to time-of-use race

Posted by Andy Shevchenko 1 month ago

On Tue, May 12, 2026 at 08:36:38PM -0500, Maxwell Doose wrote:

The Subject can be made better:

  iio: imu: kmx61: Fix potential TOCTOU race in kmx61_write_event_config()

> A time-of-check to time-of-use race condition exists in
> kmx61_write_event_config(). If two threads enter the function at the
> same time, both threads may pass the check and get to the lock. Thus,
> when the first thread releases the lock allowing the second thread to
> start execution after the first thread modifies data->ev_enable_state to
> force returning from the function, the second thread continues execution
> regardless. Fix this by moving the data->ev_enable_state check inside of
> the critical section.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2] iio: imu: kmx61: Fix potential time-of-check to time-of-use race

Posted by Maxwell Doose 1 month ago

On Wed, May 13, 2026 at 6:25 AM Andy Shevchenko
<andriy.shevchenko@intel.com> wrote:
>
> On Tue, May 12, 2026 at 08:36:38PM -0500, Maxwell Doose wrote:
>
> The Subject can be made better:
>
>   iio: imu: kmx61: Fix potential TOCTOU race in kmx61_write_event_config()
>

That all sounds good. Will wait for Jonathan to see if he just wants
to merge as is, change the subject, or ask for v3.

best regards,
max



> > A time-of-check to time-of-use race condition exists in
> > kmx61_write_event_config(). If two threads enter the function at the
> > same time, both threads may pass the check and get to the lock. Thus,
> > when the first thread releases the lock allowing the second thread to
> > start execution after the first thread modifies data->ev_enable_state to
> > force returning from the function, the second thread continues execution
> > regardless. Fix this by moving the data->ev_enable_state check inside of
> > the critical section.
>
> --
> With Best Regards,
> Andy Shevchenko
>
>

Re: [PATCH v2] iio: imu: kmx61: Fix potential time-of-check to time-of-use race

Posted by Joshua Crofts 1 month ago

On Wed, 13 May 2026 at 03:41, Maxwell Doose <m32285159@gmail.com> wrote:
>
> A time-of-check to time-of-use race condition exists in
> kmx61_write_event_config(). If two threads enter the function at the
> same time, both threads may pass the check and get to the lock. Thus,
> when the first thread releases the lock allowing the second thread to
> start execution after the first thread modifies data->ev_enable_state to
> force returning from the function, the second thread continues execution
> regardless. Fix this by moving the data->ev_enable_state check inside of
> the critical section.
>
> Fixes: fd3ae7a9f21c ("iio: imu: kmx61: Add support for any motion trigger")
> Reported-by: sashiko <sashiko-bot@kernel.org>
> Closes: https://sashiko.dev/#/patchset/20260507223337.48437-1-m32285159%40gmail.com
> Signed-off-by: Maxwell Doose <m32285159@gmail.com>
> ---
>  drivers/iio/imu/kmx61.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/iio/imu/kmx61.c b/drivers/iio/imu/kmx61.c
> index 3cd91d8a89ee..3afa369de3cf 100644
> --- a/drivers/iio/imu/kmx61.c
> +++ b/drivers/iio/imu/kmx61.c
> @@ -942,11 +942,11 @@ static int kmx61_write_event_config(struct iio_dev *indio_dev,
>         struct kmx61_data *data = kmx61_get_data(indio_dev);
>         int ret = 0;
>
> -       if (state && data->ev_enable_state)
> -               return 0;
> -
>         mutex_lock(&data->lock);
>
> +       if (state && data->ev_enable_state)
> +               goto err_unlock;
> +
>         if (!state && data->motion_trig_on) {
>                 data->ev_enable_state = false;
>                 goto err_unlock;
> --

This seems simple enough.

Reviewed-by: Joshua Crofts <joshua.crofts1@gmail.com>

-- 
Kind regards

CJD

Re: [PATCH v2] iio: imu: kmx61: Fix potential time-of-check to time-of-use race

Posted by Maxwell Doose 1 month ago

On Tue, May 12, 2026 at 8:36 PM Maxwell Doose <m32285159@gmail.com> wrote:
>
> A time-of-check to time-of-use race condition exists in
> kmx61_write_event_config(). If two threads enter the function at the
> same time, both threads may pass the check and get to the lock. Thus,
> when the first thread releases the lock allowing the second thread to
> start execution after the first thread modifies data->ev_enable_state to
> force returning from the function, the second thread continues execution
> regardless. Fix this by moving the data->ev_enable_state check inside of
> the critical section.
>
> Fixes: fd3ae7a9f21c ("iio: imu: kmx61: Add support for any motion trigger")
> Reported-by: sashiko <sashiko-bot@kernel.org>
> Closes: https://sashiko.dev/#/patchset/20260507223337.48437-1-m32285159%40gmail.com
> Signed-off-by: Maxwell Doose <m32285159@gmail.com>
> ---
>  drivers/iio/imu/kmx61.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/iio/imu/kmx61.c b/drivers/iio/imu/kmx61.c
> index 3cd91d8a89ee..3afa369de3cf 100644
> --- a/drivers/iio/imu/kmx61.c
> +++ b/drivers/iio/imu/kmx61.c
> @@ -942,11 +942,11 @@ static int kmx61_write_event_config(struct iio_dev *indio_dev,
>         struct kmx61_data *data = kmx61_get_data(indio_dev);
>         int ret = 0;
>
> -       if (state && data->ev_enable_state)
> -               return 0;
> -
>         mutex_lock(&data->lock);
>
> +       if (state && data->ev_enable_state)
> +               goto err_unlock;
> +
>         if (!state && data->motion_trig_on) {
>                 data->ev_enable_state = false;
>                 goto err_unlock;
> --
> 2.54.0
>

Silly me, forgot the changelog.

v2:
- Started from scratch due to messy git tree
- Updated commit message per Andy and Jonathan

best regards,
max