On Wed, 13 Mar 2024 10:57:35 -0500 Chenyuan Yang <chenyuan0y@xxxxxxxxx> wrote: > The sorting in iio_gts_build_avail_time_table is not working as intended. For function names, add () so The sorting in iio_gts_build_avail_time_table() is not working as intended. I can fix this up whilst applying but will be waiting for a RB or related from Matti before queuing this up. I'm assuming there are no existing drivers in tree that hit this problem? If there are reply saying which ones so I can assess whether to rush this fix in or not. Thanks, Jonathan > It could result in an out-of-bounds access when the time is zero. > > Here are more details: > > 1. When the gts->itime_table[i].time_us is zero, e.g., the time > sequence is `3, 0, 1`, the inner for-loop will not terminate and do > out-of-bound writes. This is because once `times[j] > new`, the value > `new` will be added in the current position and the `times[j]` will be > moved to `j+1` position, which makes the if-condition always hold. > Meanwhile, idx will be added one, making the loop keep running without > termination and out-of-bound write. > 2. If none of the gts->itime_table[i].time_us is zero, the elements > will just be copied without being sorted as described in the comment > "Sort times from all tables to one and remove duplicates". > > For more details, please refer to https://lore.kernel.org/all/6dd0d822-046c-4dd2-9532-79d7ab96ec05@xxxxxxxxx. > > Reported-by: Chenyuan Yang <chenyuan0y@xxxxxxxxx> > Suggested-by: Matti Vaittinen <mazziesaccount@xxxxxxxxx> > Fixes: 38416c28e16890b52fdd5eb73479299ec3f062f3 ("iio: light: Add gain-time-scale helpers") > Signed-off-by: Chenyuan Yang <chenyuan0y@xxxxxxxxx> > --- > diff --git a/drivers/iio/industrialio-gts-helper.c b/drivers/iio/industrialio-gts-helper.c > index 7653261d2dc2..61714a8d5717 100644 > --- a/drivers/iio/industrialio-gts-helper.c > +++ b/drivers/iio/industrialio-gts-helper.c > @@ -375,7 +375,7 @@ static int iio_gts_build_avail_time_table(struct iio_gts *gts) > for (i = gts->num_itime - 1; i >= 0; i--) { > int new = gts->itime_table[i].time_us; > > - if (times[idx] < new) { > + if (idx == 0 || times[idx - 1] < new) { > times[idx++] = new; > continue; > } > @@ -385,9 +385,10 @@ static int iio_gts_build_avail_time_table(struct iio_gts *gts) > memmove(×[j + 1], ×[j], > (idx - j) * sizeof(int)); > times[j] = new; > - idx++; > + break; > } > } > + idx++; > } > > /* create a list of times formatted as list of IIO_VAL_INT_PLUS_MICRO */ >
