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