[PATCH] iio: imu: kmx61: Fix TOCTOU race condition

[PATCH] iio: imu: kmx61: Fix TOCTOU race condition

[Maxwell Doose]

A Time-of-check to Time-of-use race condition is present in
kmx61_write_event_config(). Move the mutex_lock() call above it to fix
it.

Fixes: fd3ae7a9f21c ("iio: imu: kmx61: Add support for any motion trigger")
Signed-off-by: Maxwell Doose <m32285159@gmail.com>
---
 drivers/iio/imu/kmx61.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/iio/imu/kmx61.c b/drivers/iio/imu/kmx61.c
index 3cd91d8a89ee..9aa00acc7f14 100644
--- a/drivers/iio/imu/kmx61.c
+++ b/drivers/iio/imu/kmx61.c
@@ -942,11 +942,13 @@ 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) {
+		ret = 0;
+		goto err_unlock;
+	}
+
 	if (!state && data->motion_trig_on) {
 		data->ev_enable_state = false;
 		goto err_unlock;
-- 
2.54.0

Re: [PATCH] iio: imu: kmx61: Fix TOCTOU race condition

[Maxwell Doose]

On Tue May 12, 2026 at 7:03 AM CDT, Maxwell Doose wrote:
> A Time-of-check to Time-of-use race condition is present in
> kmx61_write_event_config(). Move the mutex_lock() call above it to fix
> it.
>
> Fixes: fd3ae7a9f21c ("iio: imu: kmx61: Add support for any motion trigger")
> Signed-off-by: Maxwell Doose <m32285159@gmail.com>
> ---
>  drivers/iio/imu/kmx61.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/iio/imu/kmx61.c b/drivers/iio/imu/kmx61.c
> index 3cd91d8a89ee..9aa00acc7f14 100644
> --- a/drivers/iio/imu/kmx61.c
> +++ b/drivers/iio/imu/kmx61.c
> @@ -942,11 +942,13 @@ 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) {
> +		ret = 0;
> +		goto err_unlock;
> +	}
> +
>  	if (!state && data->motion_trig_on) {
>  		data->ev_enable_state = false;
>  		goto err_unlock;

Whoops, forgot to add reported-by and closes tags, so:

Reported-by: sashiko <sashiko-bot@kernel.org>
Closes: https://sashiko.dev/#/patchset/20260507223337.48437-1-m32285159%40gmail.com

best regards,
max

Re: [PATCH] iio: imu: kmx61: Fix TOCTOU race condition

[Andy Shevchenko]

On Tue, May 12, 2026 at 6:17 PM Maxwell Doose <m32285159@gmail.com> wrote:
>
> On Tue May 12, 2026 at 7:03 AM CDT, Maxwell Doose wrote:
> > A Time-of-check to Time-of-use race condition is present in
> > kmx61_write_event_config(). Move the mutex_lock() call above it to fix
> > it.

I think you want to elaborate a bit more on this. Id est explain why
ev_enable_state needs to be protected. Not everybody is willing to go
to some site to read some AI reports and interpreted them.


> Reported-by: sashiko <sashiko-bot@kernel.org>
> Closes: https://sashiko.dev/#/patchset/20260507223337.48437-1-m32285159%40gmail.com


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] iio: imu: kmx61: Fix TOCTOU race condition

[Maxwell Doose]

On Tue, May 12, 2026 at 10:25 AM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
>
> On Tue, May 12, 2026 at 6:17 PM Maxwell Doose <m32285159@gmail.com> wrote:
> >
> > On Tue May 12, 2026 at 7:03 AM CDT, Maxwell Doose wrote:
> > > A Time-of-check to Time-of-use race condition is present in
> > > kmx61_write_event_config(). Move the mutex_lock() call above it to fix
> > > it.
>
> I think you want to elaborate a bit more on this. Id est explain why
> ev_enable_state needs to be protected. Not everybody is willing to go
> to some site to read some AI reports and interpreted them.
>

Can do that for v2. I believe that it needs to be protected since
later we set ev_enable_state to false (basically right after). Could
be wrong of course, but Jonathan did confirm the TOCTOU.

best regards,
max



>
> > Reported-by: sashiko <sashiko-bot@kernel.org>
> > Closes: https://sashiko.dev/#/patchset/20260507223337.48437-1-m32285159%40gmail.com
>
>
> --
> With Best Regards,
> Andy Shevchenko

Re: [PATCH] iio: imu: kmx61: Fix TOCTOU race condition

[David Lechner]

On 5/12/26 7:03 AM, Maxwell Doose wrote:
> A Time-of-check to Time-of-use race condition is present in
> kmx61_write_event_config(). Move the mutex_lock() call above it to fix
> it.
> 
> Fixes: fd3ae7a9f21c ("iio: imu: kmx61: Add support for any motion trigger")
> Signed-off-by: Maxwell Doose <m32285159@gmail.com>
> ---
>  drivers/iio/imu/kmx61.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/iio/imu/kmx61.c b/drivers/iio/imu/kmx61.c
> index 3cd91d8a89ee..9aa00acc7f14 100644
> --- a/drivers/iio/imu/kmx61.c
> +++ b/drivers/iio/imu/kmx61.c
> @@ -942,11 +942,13 @@ 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) {
> +		ret = 0;
> +		goto err_unlock;
> +	}
> +
>  	if (!state && data->motion_trig_on) {
>  		data->ev_enable_state = false;
>  		goto err_unlock;

There are actually 3 other drivers that have identical code
which likely need the same fix.

And in all of these, there is an write_event() callback that
reads ev_enable_state without holding the mutex that looks
suspicious too.

Re: [PATCH] iio: imu: kmx61: Fix TOCTOU race condition

[Jonathan Cameron]

On Tue, 12 May 2026 10:30:42 -0500
Maxwell Doose <m32285159@gmail.com> wrote:

> On Tue, May 12, 2026 at 10:25 AM Andy Shevchenko
> <andy.shevchenko@gmail.com> wrote:
> >
> > On Tue, May 12, 2026 at 6:17 PM Maxwell Doose <m32285159@gmail.com> wrote:  
> > >
> > > On Tue May 12, 2026 at 7:03 AM CDT, Maxwell Doose wrote:  
> > > > A Time-of-check to Time-of-use race condition is present in
> > > > kmx61_write_event_config(). Move the mutex_lock() call above it to fix
> > > > it.  
> >
> > I think you want to elaborate a bit more on this. Id est explain why
> > ev_enable_state needs to be protected. Not everybody is willing to go
> > to some site to read some AI reports and interpreted them.
> >  
> 
> Can do that for v2. I believe that it needs to be protected since
> later we set ev_enable_state to false (basically right after). Could
> be wrong of course, but Jonathan did confirm the TOCTOU.

I'd talk more about how we'd get a race.  If two calls enter the function
at the same time (which is easy to do) they may both pass this check before
getting to the lock.  Therefore we end up with at best pointless repeated
work, at worst a reference or similar count issue. You'd need to look closely
at what is protected to be sure whether it benign waste of time or a real
bug.

Jonathan

> 
> best regards,
> max
> 
> 
> 
> >  
> > > Reported-by: sashiko <sashiko-bot@kernel.org>
> > > Closes: https://sashiko.dev/#/patchset/20260507223337.48437-1-m32285159%40gmail.com  
> >
> >
> > --
> > With Best Regards,
> > Andy Shevchenko  

Re: [PATCH] iio: imu: kmx61: Fix TOCTOU race condition

[Maxwell Doose]

On Tue, May 12, 2026 at 12:10 PM Jonathan Cameron <jic23@kernel.org> wrote:
>
> On Tue, 12 May 2026 10:30:42 -0500
> Maxwell Doose <m32285159@gmail.com> wrote:
>
> > On Tue, May 12, 2026 at 10:25 AM Andy Shevchenko
> > <andy.shevchenko@gmail.com> wrote:
> > >
> > > On Tue, May 12, 2026 at 6:17 PM Maxwell Doose <m32285159@gmail.com> wrote:
> > > >
> > > > On Tue May 12, 2026 at 7:03 AM CDT, Maxwell Doose wrote:
> > > > > A Time-of-check to Time-of-use race condition is present in
> > > > > kmx61_write_event_config(). Move the mutex_lock() call above it to fix
> > > > > it.
> > >
> > > I think you want to elaborate a bit more on this. Id est explain why
> > > ev_enable_state needs to be protected. Not everybody is willing to go
> > > to some site to read some AI reports and interpreted them.
> > >
> >
> > Can do that for v2. I believe that it needs to be protected since
> > later we set ev_enable_state to false (basically right after). Could
> > be wrong of course, but Jonathan did confirm the TOCTOU.
>
> I'd talk more about how we'd get a race.  If two calls enter the function
> at the same time (which is easy to do) they may both pass this check before
> getting to the lock.  Therefore we end up with at best pointless repeated
> work, at worst a reference or similar count issue. You'd need to look closely
> at what is protected to be sure whether it benign waste of time or a real
> bug.
>

Well, since we're accessing the shared state via kmx61_get_data (by
the way of iio_priv), we could check that value, pass the check, and
then have the value change before we acquire the lock. TOCTOU race,
no? If data->ev_enable_state is false then becomes true after we check
the value but before we get the lock, then ev_enable_state changes to
whatever the input variable state is. Anyways, just saying that this
is certainly a bug. Would coming across it be common? Probably not,
likely one of the much rarer ones, but still worth fixing.

best regards,
max




> Jonathan
>
> >
> > best regards,
> > max
> >
> >
> >
> > >
> > > > Reported-by: sashiko <sashiko-bot@kernel.org>
> > > > Closes: https://sashiko.dev/#/patchset/20260507223337.48437-1-m32285159%40gmail.com
> > >
> > >
> > > --
> > > With Best Regards,
> > > Andy Shevchenko
>

Re: [PATCH] iio: imu: kmx61: Fix TOCTOU race condition

[Jonathan Cameron]

On Tue, 12 May 2026 12:37:56 -0500
Maxwell Doose <m32285159@gmail.com> wrote:

> On Tue, May 12, 2026 at 12:10 PM Jonathan Cameron <jic23@kernel.org> wrote:
> >
> > On Tue, 12 May 2026 10:30:42 -0500
> > Maxwell Doose <m32285159@gmail.com> wrote:
> >  
> > > On Tue, May 12, 2026 at 10:25 AM Andy Shevchenko
> > > <andy.shevchenko@gmail.com> wrote:  
> > > >
> > > > On Tue, May 12, 2026 at 6:17 PM Maxwell Doose <m32285159@gmail.com> wrote:  
> > > > >
> > > > > On Tue May 12, 2026 at 7:03 AM CDT, Maxwell Doose wrote:  
> > > > > > A Time-of-check to Time-of-use race condition is present in
> > > > > > kmx61_write_event_config(). Move the mutex_lock() call above it to fix
> > > > > > it.  
> > > >
> > > > I think you want to elaborate a bit more on this. Id est explain why
> > > > ev_enable_state needs to be protected. Not everybody is willing to go
> > > > to some site to read some AI reports and interpreted them.
> > > >  
> > >
> > > Can do that for v2. I believe that it needs to be protected since
> > > later we set ev_enable_state to false (basically right after). Could
> > > be wrong of course, but Jonathan did confirm the TOCTOU.  
> >
> > I'd talk more about how we'd get a race.  If two calls enter the function
> > at the same time (which is easy to do) they may both pass this check before
> > getting to the lock.  Therefore we end up with at best pointless repeated
> > work, at worst a reference or similar count issue. You'd need to look closely
> > at what is protected to be sure whether it benign waste of time or a real
> > bug.
> >  
> 
> Well, since we're accessing the shared state via kmx61_get_data (by
> the way of iio_priv), we could check that value, pass the check, and
> then have the value change before we acquire the lock. TOCTOU race,
> no? If data->ev_enable_state is false then becomes true after we check
> the value but before we get the lock, then ev_enable_state changes to
> whatever the input variable state is. Anyways, just saying that this
> is certainly a bug. Would coming across it be common? Probably not,
> likely one of the much rarer ones, but still worth fixing.

It's not a bug if for instance two racing value sets result
in any random one of them being written.  That happens even with the
locking. It is only a bug if letting both of them write results
in something overflowing.  Some writes in cases like this are idempotent
- e.g. turning something on that is already on has no affect.

My guess would be that the power state transition could happen twice
in the enable direction resulting in +2 rather than +1 resulting in
a reference counter that may never go back to 0.

Let's look at the code and the various state combinations.
Ignore motion_trig_on for now.

state == true and data->enable_state == false

static int kmx61_write_event_config(struct iio_dev *indio_dev,
				    const struct iio_chan_spec *chan,
				    enum iio_event_type type,
				    enum iio_event_direction dir,
				    bool state)
{
	struct kmx61_data *data = kmx61_get_data(indio_dev);
	int ret = 0;

	if (state && data->ev_enable_state)
// Both threads pass this check
		return 0;

// Threads serialized in next section
	mutex_lock(&data->lock);

//ignore for simplicity - though I'm far from convinced this
//is right either for the state == false path when
//motion_trig_on == true - I think the runtime pm counters
// will not be decremented whereas they were incremented
// on the state == true, motion_trig_on = true path.
//	if (!state && data->motion_trig_on) {
//		data->ev_enable_state = false;
//		goto err_unlock;
//	}


//Next bit runs twice and in there we pm_runtime_resume_and_get()
//which will run twice for the same thing being enabled - hence the +2
//reference count mentioned above.
	ret = kmx61_set_power_state(data, state, KMX61_ACC);
	if (ret < 0)
		goto err_unlock;

	ret = kmx61_setup_any_motion_interrupt(data, state);
	if (ret < 0) {
		kmx61_set_power_state(data, false, KMX61_ACC);
		goto err_unlock;
	}

	data->ev_enable_state = state;

err_unlock:
	mutex_unlock(&data->lock);

	return ret;
}

Anyhow, looks like a nest of race conditions.  If you don't mind
could you take a look more generally at the state machines in here
and see if we have more bugs to close?  That will also help you
describe why this particular fix is right.

Jonathan

> 
> best regards,
> max
> 
> 
> 
> 
> > Jonathan
> >  
> > >
> > > best regards,
> > > max
> > >
> > >
> > >  
> > > >  
> > > > > Reported-by: sashiko <sashiko-bot@kernel.org>
> > > > > Closes: https://sashiko.dev/#/patchset/20260507223337.48437-1-m32285159%40gmail.com  
> > > >
> > > >
> > > > --
> > > > With Best Regards,
> > > > Andy Shevchenko  
> >