1. Patchew
  2. linux
  3. [PATCH 0/3] some objtool warnings fixes
  4. View patch

[PATCH 3/3] iio: imu: bmi160: Remove potential undefined behavior in bmi160_config_pin()

Josh Poimboeuf posted 3 patches 1 month ago
[PATCH 3/3] iio: imu: bmi160: Remove potential undefined behavior in bmi160_config_pin()
Posted by Josh Poimboeuf 1 month ago 
If 'pin' is not one of its expected values, the value of
'int_out_ctrl_shift' is undefined.  With UBSAN enabled, this causes
Clang to generate undefined behavior, resulting in the following
warning:

  drivers/iio/imu/bmi160/bmi160_core.o: warning: objtool: bmi160_setup_irq() falls through to next function __cfi_bmi160_core_runtime_resume()

Prevent the UB and improve error handling by adding a BUG() if 'pin' has
an unexpected value.

Cc: Jonathan Cameron <jic23@kernel.org>
Cc: David Lechner <dlechner@baylibre.com>
CC: Nuno Sá <nuno.sa@analog.com>
Cc: Andy Shevchenko <andy@kernel.org>
Fixes: 895bf81e6bbf ("iio:bmi160: add drdy interrupt support")
Reported-by: Arnd Bergmann <arnd@arndb.de>
Closes: https://lore.kernel.org/a426d669-58bb-4be1-9eaa-6f3d83109e2d@app.fastmail.com
Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
---
 drivers/iio/imu/bmi160/bmi160_core.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/iio/imu/bmi160/bmi160_core.c b/drivers/iio/imu/bmi160/bmi160_core.c
index 5f47708b4c5d..e5326df75e49 100644
--- a/drivers/iio/imu/bmi160/bmi160_core.c
+++ b/drivers/iio/imu/bmi160/bmi160_core.c
@@ -579,6 +579,8 @@ static int bmi160_config_pin(struct regmap *regmap, enum bmi160_int_pin pin,
 		int_latch_mask = BMI160_INT2_LATCH_MASK;
 		int_map_mask = BMI160_INT2_MAP_DRDY_EN;
 		break;
+	default:
+		BUG();
 	}
 	int_out_ctrl_mask = BMI160_INT_OUT_CTRL_MASK << int_out_ctrl_shift;
 
-- 
2.53.0
Re: [PATCH 3/3] iio: imu: bmi160: Remove potential undefined behavior in bmi160_config_pin()
Posted by Andy Shevchenko 1 month ago 
On Mon, Mar 9, 2026 at 6:03 PM Josh Poimboeuf <jpoimboe@kernel.org> wrote:
>
> If 'pin' is not one of its expected values, the value of
> 'int_out_ctrl_shift' is undefined.  With UBSAN enabled, this causes
> Clang to generate undefined behavior, resulting in the following
> warning:
>
>   drivers/iio/imu/bmi160/bmi160_core.o: warning: objtool: bmi160_setup_irq() falls through to next function __cfi_bmi160_core_runtime_resume()
>
> Prevent the UB and improve error handling by adding a BUG() if 'pin' has
> an unexpected value.

> Cc: Jonathan Cameron <jic23@kernel.org>
> Cc: David Lechner <dlechner@baylibre.com>
> CC: Nuno Sá <nuno.sa@analog.com>
> Cc: Andy Shevchenko <andy@kernel.org>

Please, reduce the noise in the commit message by moving Cc list to be
below the '---' line. This will have the same effect on email (Git
tools work fine) and if anybody needs this information it will be
preserved in lore.kernel.org archive. In case you have questions about
handling this locally, there is a subthread (patch 18) of this series:
https://lore.kernel.org/lkml/20260123113708.416727-19-bigeasy@linutronix.de/

-- 
With Best Regards,
Andy Shevchenko
Re: [PATCH 3/3] iio: imu: bmi160: Remove potential undefined behavior in bmi160_config_pin()
Posted by Josh Poimboeuf 1 month ago 
On Mon, Mar 09, 2026 at 06:40:26PM +0200, Andy Shevchenko wrote:
> On Mon, Mar 9, 2026 at 6:03 PM Josh Poimboeuf <jpoimboe@kernel.org> wrote:
> >
> > If 'pin' is not one of its expected values, the value of
> > 'int_out_ctrl_shift' is undefined.  With UBSAN enabled, this causes
> > Clang to generate undefined behavior, resulting in the following
> > warning:
> >
> >   drivers/iio/imu/bmi160/bmi160_core.o: warning: objtool: bmi160_setup_irq() falls through to next function __cfi_bmi160_core_runtime_resume()
> >
> > Prevent the UB and improve error handling by adding a BUG() if 'pin' has
> > an unexpected value.
> 
> > Cc: Jonathan Cameron <jic23@kernel.org>
> > Cc: David Lechner <dlechner@baylibre.com>
> > CC: Nuno Sá <nuno.sa@analog.com>
> > Cc: Andy Shevchenko <andy@kernel.org>
> 
> Please, reduce the noise in the commit message by moving Cc list to be
> below the '---' line. This will have the same effect on email (Git
> tools work fine) and if anybody needs this information it will be
> preserved in lore.kernel.org archive. In case you have questions about
> handling this locally, there is a subthread (patch 18) of this series:
> https://lore.kernel.org/lkml/20260123113708.416727-19-bigeasy@linutronix.de/

Will do, thanks.

-- 
Josh
Re: [PATCH 3/3] iio: imu: bmi160: Remove potential undefined behavior in bmi160_config_pin()
Posted by David Lechner 1 month ago 
On 3/9/26 11:40 AM, Andy Shevchenko wrote:
> On Mon, Mar 9, 2026 at 6:03 PM Josh Poimboeuf <jpoimboe@kernel.org> wrote:
>>
>> If 'pin' is not one of its expected values, the value of
>> 'int_out_ctrl_shift' is undefined.  With UBSAN enabled, this causes
>> Clang to generate undefined behavior, resulting in the following
>> warning:
>>
>>   drivers/iio/imu/bmi160/bmi160_core.o: warning: objtool: bmi160_setup_irq() falls through to next function __cfi_bmi160_core_runtime_resume()
>>
>> Prevent the UB and improve error handling by adding a BUG() if 'pin' has
>> an unexpected value.
> 
>> Cc: Jonathan Cameron <jic23@kernel.org>
>> Cc: David Lechner <dlechner@baylibre.com>
>> CC: Nuno Sá <nuno.sa@analog.com>
>> Cc: Andy Shevchenko <andy@kernel.org>
> 
> Please, reduce the noise in the commit message by moving Cc list to be
> below the '---' line. This will have the same effect on email (Git
> tools work fine) and if anybody needs this information it will be
> preserved in lore.kernel.org archive. In case you have questions about
> handling this locally, there is a subthread (patch 18) of this series:
> https://lore.kernel.org/lkml/20260123113708.416727-19-bigeasy@linutronix.de/
> 

And be sure to include the IIO mailing list too, not just the maintainers/
reviewers. Omitting it breaks some workflows.
Re: [PATCH 3/3] iio: imu: bmi160: Remove potential undefined behavior in bmi160_config_pin()
Posted by Nuno Sá 1 month ago 
On Mon, 2026-03-09 at 09:03 -0700, Josh Poimboeuf wrote:
> If 'pin' is not one of its expected values, the value of
> 'int_out_ctrl_shift' is undefined.  With UBSAN enabled, this causes
> Clang to generate undefined behavior, resulting in the following
> warning:
> 
>   drivers/iio/imu/bmi160/bmi160_core.o: warning: objtool: bmi160_setup_irq() falls through to next
> function __cfi_bmi160_core_runtime_resume()
> 
> Prevent the UB and improve error handling by adding a BUG() if 'pin' has
> an unexpected value.
> 
> Cc: Jonathan Cameron <jic23@kernel.org>
> Cc: David Lechner <dlechner@baylibre.com>
> CC: Nuno Sá <nuno.sa@analog.com>
> Cc: Andy Shevchenko <andy@kernel.org>
> Fixes: 895bf81e6bbf ("iio:bmi160: add drdy interrupt support")
> Reported-by: Arnd Bergmann <arnd@arndb.de>
> Closes: https://lore.kernel.org/a426d669-58bb-4be1-9eaa-6f3d83109e2d@app.fastmail.com
> Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
> ---
>  drivers/iio/imu/bmi160/bmi160_core.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/iio/imu/bmi160/bmi160_core.c b/drivers/iio/imu/bmi160/bmi160_core.c
> index 5f47708b4c5d..e5326df75e49 100644
> --- a/drivers/iio/imu/bmi160/bmi160_core.c
> +++ b/drivers/iio/imu/bmi160/bmi160_core.c
> @@ -579,6 +579,8 @@ static int bmi160_config_pin(struct regmap *regmap, enum bmi160_int_pin pin,
>  		int_latch_mask = BMI160_INT2_LATCH_MASK;
>  		int_map_mask = BMI160_INT2_MAP_DRDY_EN;
>  		break;
> +	default:
> +		BUG();
>  	}
>  	int_out_ctrl_mask = BMI160_INT_OUT_CTRL_MASK << int_out_ctrl_shift;
>  

AFAIK, BUG() is not something we should use lightly so I wonder why having it rather that a normal
'return -EINVAL'?

At the very least, it could be WARN but I still think that's too much for a device .probe(). Any
special reason using BUG()?

Also seems like:

https://elixir.bootlin.com/linux/v7.0-rc2/source/drivers/iio/imu/bmi160/bmi160_core.c#L624

could be improved? Like setting 'pin_name' in the first switch() case.

- Nuno Sá
Re: [PATCH 3/3] iio: imu: bmi160: Remove potential undefined behavior in bmi160_config_pin()
Posted by Josh Poimboeuf 1 month ago 
On Mon, Mar 09, 2026 at 04:16:16PM +0000, Nuno Sá wrote:
> On Mon, 2026-03-09 at 09:03 -0700, Josh Poimboeuf wrote:
> > If 'pin' is not one of its expected values, the value of
> > 'int_out_ctrl_shift' is undefined.  With UBSAN enabled, this causes
> > Clang to generate undefined behavior, resulting in the following
> > warning:
> > 
> >   drivers/iio/imu/bmi160/bmi160_core.o: warning: objtool: bmi160_setup_irq() falls through to next
> > function __cfi_bmi160_core_runtime_resume()
> > 
> > Prevent the UB and improve error handling by adding a BUG() if 'pin' has
> > an unexpected value.
> > 
> > Cc: Jonathan Cameron <jic23@kernel.org>
> > Cc: David Lechner <dlechner@baylibre.com>
> > CC: Nuno Sá <nuno.sa@analog.com>
> > Cc: Andy Shevchenko <andy@kernel.org>
> > Fixes: 895bf81e6bbf ("iio:bmi160: add drdy interrupt support")
> > Reported-by: Arnd Bergmann <arnd@arndb.de>
> > Closes: https://lore.kernel.org/a426d669-58bb-4be1-9eaa-6f3d83109e2d@app.fastmail.com
> > Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
> > ---
> >  drivers/iio/imu/bmi160/bmi160_core.c | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/drivers/iio/imu/bmi160/bmi160_core.c b/drivers/iio/imu/bmi160/bmi160_core.c
> > index 5f47708b4c5d..e5326df75e49 100644
> > --- a/drivers/iio/imu/bmi160/bmi160_core.c
> > +++ b/drivers/iio/imu/bmi160/bmi160_core.c
> > @@ -579,6 +579,8 @@ static int bmi160_config_pin(struct regmap *regmap, enum bmi160_int_pin pin,
> >  		int_latch_mask = BMI160_INT2_LATCH_MASK;
> >  		int_map_mask = BMI160_INT2_MAP_DRDY_EN;
> >  		break;
> > +	default:
> > +		BUG();
> >  	}
> >  	int_out_ctrl_mask = BMI160_INT_OUT_CTRL_MASK << int_out_ctrl_shift;
> >  
> 
> AFAIK, BUG() is not something we should use lightly so I wonder why having it rather that a normal
> 'return -EINVAL'?
> 
> At the very least, it could be WARN but I still think that's too much for a device .probe(). Any
> special reason using BUG()?

Using BUG() for a default "invalid enum" is a common pattern, but indeed
returning an error would also be valid here.  I can change that to
return -EINVAL.

> Also seems like:
> 
> https://elixir.bootlin.com/linux/v7.0-rc2/source/drivers/iio/imu/bmi160/bmi160_core.c#L624
> 
> could be improved? Like setting 'pin_name' in the first switch() case.

Yeah, I assume you mean setting 'pin_name' to something like "invalid"
for the default case?

-- 
Josh
Re: [PATCH 3/3] iio: imu: bmi160: Remove potential undefined behavior in bmi160_config_pin()
Posted by Nuno Sá 1 month ago 
On Mon, 2026-03-09 at 09:27 -0700, Josh Poimboeuf wrote:
> On Mon, Mar 09, 2026 at 04:16:16PM +0000, Nuno Sá wrote:
> > On Mon, 2026-03-09 at 09:03 -0700, Josh Poimboeuf wrote:
> > > If 'pin' is not one of its expected values, the value of
> > > 'int_out_ctrl_shift' is undefined.  With UBSAN enabled, this causes
> > > Clang to generate undefined behavior, resulting in the following
> > > warning:
> > > 
> > >   drivers/iio/imu/bmi160/bmi160_core.o: warning: objtool: bmi160_setup_irq() falls through to
> > > next
> > > function __cfi_bmi160_core_runtime_resume()
> > > 
> > > Prevent the UB and improve error handling by adding a BUG() if 'pin' has
> > > an unexpected value.
> > > 
> > > Cc: Jonathan Cameron <jic23@kernel.org>
> > > Cc: David Lechner <dlechner@baylibre.com>
> > > CC: Nuno Sá <nuno.sa@analog.com>
> > > Cc: Andy Shevchenko <andy@kernel.org>
> > > Fixes: 895bf81e6bbf ("iio:bmi160: add drdy interrupt support")
> > > Reported-by: Arnd Bergmann <arnd@arndb.de>
> > > Closes: https://lore.kernel.org/a426d669-58bb-4be1-9eaa-6f3d83109e2d@app.fastmail.com
> > > Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
> > > ---
> > >  drivers/iio/imu/bmi160/bmi160_core.c | 2 ++
> > >  1 file changed, 2 insertions(+)
> > > 
> > > diff --git a/drivers/iio/imu/bmi160/bmi160_core.c b/drivers/iio/imu/bmi160/bmi160_core.c
> > > index 5f47708b4c5d..e5326df75e49 100644
> > > --- a/drivers/iio/imu/bmi160/bmi160_core.c
> > > +++ b/drivers/iio/imu/bmi160/bmi160_core.c
> > > @@ -579,6 +579,8 @@ static int bmi160_config_pin(struct regmap *regmap, enum bmi160_int_pin
> > > pin,
> > >  		int_latch_mask = BMI160_INT2_LATCH_MASK;
> > >  		int_map_mask = BMI160_INT2_MAP_DRDY_EN;
> > >  		break;
> > > +	default:
> > > +		BUG();
> > >  	}
> > >  	int_out_ctrl_mask = BMI160_INT_OUT_CTRL_MASK << int_out_ctrl_shift;
> > >  
> > 
> > AFAIK, BUG() is not something we should use lightly so I wonder why having it rather that a
> > normal
> > 'return -EINVAL'?
> > 
> > At the very least, it could be WARN but I still think that's too much for a device .probe(). Any
> > special reason using BUG()?
> 
> Using BUG() for a default "invalid enum" is a common pattern, but indeed
> returning an error would also be valid here.  I can change that to
> return -EINVAL.
> 
> > Also seems like:
> > 
> > https://elixir.bootlin.com/linux/v7.0-rc2/source/drivers/iio/imu/bmi160/bmi160_core.c#L624
> > 
> > could be improved? Like setting 'pin_name' in the first switch() case.
> 
> Yeah, I assume you mean setting 'pin_name' to something like "invalid"
> for the default case?

Nope, I meant not duplicating the switch() case. The default should return an error so I would say
no need to bother in setting the name for that case.

- Nuno Sá
Re: [PATCH 3/3] iio: imu: bmi160: Remove potential undefined behavior in bmi160_config_pin()
Posted by Josh Poimboeuf 1 month ago 
On Mon, Mar 09, 2026 at 04:31:49PM +0000, Nuno Sá wrote:
> On Mon, 2026-03-09 at 09:27 -0700, Josh Poimboeuf wrote:
> > On Mon, Mar 09, 2026 at 04:16:16PM +0000, Nuno Sá wrote:
> > > On Mon, 2026-03-09 at 09:03 -0700, Josh Poimboeuf wrote:
> > > > If 'pin' is not one of its expected values, the value of
> > > > 'int_out_ctrl_shift' is undefined.  With UBSAN enabled, this causes
> > > > Clang to generate undefined behavior, resulting in the following
> > > > warning:
> > > > 
> > > >   drivers/iio/imu/bmi160/bmi160_core.o: warning: objtool: bmi160_setup_irq() falls through to
> > > > next
> > > > function __cfi_bmi160_core_runtime_resume()
> > > > 
> > > > Prevent the UB and improve error handling by adding a BUG() if 'pin' has
> > > > an unexpected value.
> > > > 
> > > > Cc: Jonathan Cameron <jic23@kernel.org>
> > > > Cc: David Lechner <dlechner@baylibre.com>
> > > > CC: Nuno Sá <nuno.sa@analog.com>
> > > > Cc: Andy Shevchenko <andy@kernel.org>
> > > > Fixes: 895bf81e6bbf ("iio:bmi160: add drdy interrupt support")
> > > > Reported-by: Arnd Bergmann <arnd@arndb.de>
> > > > Closes: https://lore.kernel.org/a426d669-58bb-4be1-9eaa-6f3d83109e2d@app.fastmail.com
> > > > Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
> > > > ---
> > > >  drivers/iio/imu/bmi160/bmi160_core.c | 2 ++
> > > >  1 file changed, 2 insertions(+)
> > > > 
> > > > diff --git a/drivers/iio/imu/bmi160/bmi160_core.c b/drivers/iio/imu/bmi160/bmi160_core.c
> > > > index 5f47708b4c5d..e5326df75e49 100644
> > > > --- a/drivers/iio/imu/bmi160/bmi160_core.c
> > > > +++ b/drivers/iio/imu/bmi160/bmi160_core.c
> > > > @@ -579,6 +579,8 @@ static int bmi160_config_pin(struct regmap *regmap, enum bmi160_int_pin
> > > > pin,
> > > >  		int_latch_mask = BMI160_INT2_LATCH_MASK;
> > > >  		int_map_mask = BMI160_INT2_MAP_DRDY_EN;
> > > >  		break;
> > > > +	default:
> > > > +		BUG();
> > > >  	}
> > > >  	int_out_ctrl_mask = BMI160_INT_OUT_CTRL_MASK << int_out_ctrl_shift;
> > > >  
> > > 
> > > AFAIK, BUG() is not something we should use lightly so I wonder why having it rather that a
> > > normal
> > > 'return -EINVAL'?
> > > 
> > > At the very least, it could be WARN but I still think that's too much for a device .probe(). Any
> > > special reason using BUG()?
> > 
> > Using BUG() for a default "invalid enum" is a common pattern, but indeed
> > returning an error would also be valid here.  I can change that to
> > return -EINVAL.
> > 
> > > Also seems like:
> > > 
> > > https://elixir.bootlin.com/linux/v7.0-rc2/source/drivers/iio/imu/bmi160/bmi160_core.c#L624
> > > 
> > > could be improved? Like setting 'pin_name' in the first switch() case.
> > 
> > Yeah, I assume you mean setting 'pin_name' to something like "invalid"
> > for the default case?
> 
> Nope, I meant not duplicating the switch() case. The default should return an error so I would say
> no need to bother in setting the name for that case.

Ah, you mean combining the switch() statements into one.  Ok, I'll do
that.

-- 
Josh

Patchew 2.1-devel-ea86937

© 2016 - 2026 Red Hat, Inc.