Re: [PATCH] iio: imu: inv_icm42600: fix 64bit multiplication

[Antonio Quartulli <antonio@mandelbit.com>]

On 17/07/2025 08:21, Andy Shevchenko wrote:
> On Wed, Jul 16, 2025 at 10:44 PM Antonio Quartulli
> <antonio@mandelbit.com> wrote:
>> On 16/07/2025 21:36, Andy Shevchenko wrote:
>>> On Wed, Jul 16, 2025 at 10:22 PM Antonio Quartulli
>>> <antonio@mandelbit.com> wrote:
>>>>
>>>> In inv_icm42600_accel_convert_wom_to_roc() multiplying
>>>> `threshold` by `convert` may result in a number requiring more
>>>> than 32bit.
>>>> In this case, although `value` is 64bit wide, the result is
>>>> truncated because the multiplication is performed in the
>>>> 32bit domain, due to both operands being 32bit long.
>>>>
>>>> Cast the first operand to u64 to ensure the multiplication is
>>>> performed in the expected domain.
>>>
>>> Is this a theoretical or practical issue?
>>
>> Can't say if it's practical because I don't know how large `threshold`
>> can be.
>>
>> This said, `value` is declared as u64, therefore I assumed the result is
>> expected to be potentially larger than 32 bits.
> 
> Yeah, but can you prove the potential overflow? Can the compiler prove
> otherwise?

As David pointed out, threshold is limited to [1, 255], as returned by 
inv_icm42600_accel_convert_roc_to_wom(), therefore no real overflow is 
possible.

It's the variable types being used that didn't make that fully clear.
Maybe this could be cleaned up a little, but there is nothing to "fix".

> 
>>>> Address-Coverity-ID: 1647596 ("Integer handling issues (OVERFLOW_BEFORE_WIDEN)")
>>>
>>> Is this tag now official? I can't find where it's documented.
>>
>> I don't think it's official, but it's what I have found in the git
>> history for other bugs reported by Coverity.
>> I already used it on other accepted patches as well.
> 
> Are you an official representative from the coverity that runs against
> Linux source code? Or please explain how it keeps those numbers (e.g.,
> 1647596) unique per project. For example, you and I run coverity and
> we get let's say the very same number (ID) but for different problems
> (even if we are looking at the same project), is this scenario real?
> If not, we need to have documentation inside the Linux kernel source
> tree explaining how to run Coverity, who is eligible to do that, how
> to resolve clashes (if any), etc., etc...

There is an ("official") linux-next project in Coverity managed by 
Gustavo A. R. Silva, where all maintainers can subscribe to and then 
receive the weekly scan results:

https://scan.coverity.com/projects/linux-next-weekly-scan
https://embeddedor.com/blog/2024/09/28/one-simple-and-rewarding-way-to-contribute-to-the-linux-kernel-fix-coverity-issues/

I assumed all IDs in git are taken from that scan/project.

Regards,

-- 
Antonio Quartulli

CEO and Co-Founder
Mandelbit Srl
https://www.mandelbit.com