On Mon, 4 Nov 2024 16:26:31 +0000 Karan Sanghavi <karansanghvi98@xxxxxxxxx> wrote: > On Sun, Nov 03, 2024 at 11:18:27AM +0000, Jonathan Cameron wrote: > > On Sun, 03 Nov 2024 08:43:14 +0000 > > Karan Sanghavi <karansanghvi98@xxxxxxxxx> wrote: > > > > Hi Karan, > > > > > Typecast a variable to int64_t for 64-bit arithmetic multiplication > > > > The path to actually triggering this is non obvious as these > > inputs are the result of rather complex code paths and per chip > > constraints. Have you identified a particular combination that overflows > > or is this just based on the type? I have no problem with applying this > > as hardening against future uses but unless we have a path to trigger > > it today it isn't a fix. > > > > If you do have a path, this description should state what it is. > > > > The above issue is discovered by Coverity with CID 1586045 and 1586044. > Link: https://scan7.scan.coverity.com/#/project-view/51946/11354?selectedIssue=1586045 > > Should I mention this path in the commit short message? That wasn't what I meant. I was after what combination of possible inputs actually trigger this rather than (I suspect) local analysis coverity has done. > > > > > > > Signed-off-by: Karan Sanghavi <karansanghvi98@xxxxxxxxx> > > If it's a real bug, needs a Fixes tag so we know how far to backport it. > > > > What kind of Fixes tag should I provide here. The patch that introduced the bug in the first place. See submitting patches docs for the format. However, I suspect this is coverity firing a false positive be it a reasonable one that we should tidy up. As such I'll queue this patch up, but not as a fix that I'm rushing in, but just as general cleanup for next cycle. If you find a path to trigger the overflow via userspace inputs etc then I'm happy to move it over to being handled as an urgent fix. Jonathan > > > > --- > > > drivers/iio/common/inv_sensors/inv_sensors_timestamp.c | 4 ++-- > > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > > > diff --git a/drivers/iio/common/inv_sensors/inv_sensors_timestamp.c b/drivers/iio/common/inv_sensors/inv_sensors_timestamp.c > > > index f44458c380d9..d1d11d0b2458 100644 > > > --- a/drivers/iio/common/inv_sensors/inv_sensors_timestamp.c > > > +++ b/drivers/iio/common/inv_sensors/inv_sensors_timestamp.c > > > @@ -105,8 +105,8 @@ static bool inv_update_chip_period(struct inv_sensors_timestamp *ts, > > > > > > static void inv_align_timestamp_it(struct inv_sensors_timestamp *ts) > > > { > > > - const int64_t period_min = ts->min_period * ts->mult; > > > - const int64_t period_max = ts->max_period * ts->mult; > > > + const int64_t period_min = (int64_t)ts->min_period * ts->mult; > > > + const int64_t period_max = (int64_t)ts->max_period * ts->mult; > > > int64_t add_max, sub_max; > > > int64_t delta, jitter; > > > int64_t adjust; > > > > > > --- > > > base-commit: 81983758430957d9a5cb3333fe324fd70cf63e7e > > > change-id: 20241102-coverity1586045integeroverflow-cbbf357475d9 > > > > > > Best regards, > > > > Thank you, > Karan.
