| Message ID | Y0kuaO9PQkSQja+A@kili (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | iio: imu: bno055: uninitialized variable bug in bno055_trigger_handler() | expand |
> -----Original Message----- > From: Dan Carpenter <dan.carpenter@oracle.com> > Sent: Friday, October 14, 2022 11:40 AM > To: Jonathan Cameron <jic23@kernel.org>; Andrea Merello > <andrea.merello@iit.it> > Cc: Lars-Peter Clausen <lars@metafoo.de>; linux-iio@vger.kernel.org; > kernel-janitors@vger.kernel.org > Subject: [PATCH] iio: imu: bno055: uninitialized variable bug in > bno055_trigger_handler() > > [External] > > This bug is basically harmless, although it will trigger a runtime warning > if you use KMSan. On the first iteration through the loop, the > "best_delta" variable is uninitialized so re-order the condition to > prevent reading uninitialized memory. > > Fixes: 4aefe1c2bd0c ("iio: imu: add Bosch Sensortec BNO055 core driver") > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> > --- Acked-by: Nuno Sá <nuno.sa@analog.com>
On Fri, 14 Oct 2022 12:39:52 +0300 Dan Carpenter <dan.carpenter@oracle.com> wrote: > This bug is basically harmless, although it will trigger a runtime warning > if you use KMSan. On the first iteration through the loop, the > "best_delta" variable is uninitialized so re-order the condition to > prevent reading uninitialized memory. > > Fixes: 4aefe1c2bd0c ("iio: imu: add Bosch Sensortec BNO055 core driver") > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> You reported this a while back along with a second issue (false positive) with hwval. I posted a patch fixing both https://lore.kernel.org/linux-iio/20221002145324.3776484-1-jic23@kernel.org/ I don't really care which patch goes in, but curious to reasoning to not also deal with the hwval warning here? Jonathan > --- > drivers/iio/imu/bno055/bno055.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/iio/imu/bno055/bno055.c b/drivers/iio/imu/bno055/bno055.c > index 307557a609e3..52744dd98e65 100644 > --- a/drivers/iio/imu/bno055/bno055.c > +++ b/drivers/iio/imu/bno055/bno055.c > @@ -632,7 +632,7 @@ static int bno055_set_regmask(struct bno055_priv *priv, int val, int val2, > return -EINVAL; > } > delta = abs(tbl_val - req_val); > - if (delta < best_delta || first) { > + if (first || delta < best_delta) { > best_delta = delta; > hwval = i; > first = false;
On Sat, 15 Oct 2022 17:33:59 +0100 Jonathan Cameron <jic23@kernel.org> wrote: > On Fri, 14 Oct 2022 12:39:52 +0300 > Dan Carpenter <dan.carpenter@oracle.com> wrote: > > > This bug is basically harmless, although it will trigger a runtime warning > > if you use KMSan. On the first iteration through the loop, the > > "best_delta" variable is uninitialized so re-order the condition to > > prevent reading uninitialized memory. > > > > Fixes: 4aefe1c2bd0c ("iio: imu: add Bosch Sensortec BNO055 core driver") > > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> > > You reported this a while back along with a second issue (false positive) > with hwval. > > I posted a patch fixing both > https://lore.kernel.org/linux-iio/20221002145324.3776484-1-jic23@kernel.org/ > > I don't really care which patch goes in, but curious to reasoning to not also > deal with the hwval warning here? > Meh. Rather than not applying either patch, I'll pick this one up. We can deal with the hwval warning at a later date if necessary. Applied to the fixes-togreg branch of iio.git. Jonathan > Jonathan > > > --- > > drivers/iio/imu/bno055/bno055.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/iio/imu/bno055/bno055.c b/drivers/iio/imu/bno055/bno055.c > > index 307557a609e3..52744dd98e65 100644 > > --- a/drivers/iio/imu/bno055/bno055.c > > +++ b/drivers/iio/imu/bno055/bno055.c > > @@ -632,7 +632,7 @@ static int bno055_set_regmask(struct bno055_priv *priv, int val, int val2, > > return -EINVAL; > > } > > delta = abs(tbl_val - req_val); > > - if (delta < best_delta || first) { > > + if (first || delta < best_delta) { > > best_delta = delta; > > hwval = i; > > first = false; >
On Sat, Oct 29, 2022 at 04:39:56PM +0100, Jonathan Cameron wrote: > On Sat, 15 Oct 2022 17:33:59 +0100 > Jonathan Cameron <jic23@kernel.org> wrote: > > > On Fri, 14 Oct 2022 12:39:52 +0300 > > Dan Carpenter <dan.carpenter@oracle.com> wrote: > > > > > This bug is basically harmless, although it will trigger a runtime warning > > > if you use KMSan. On the first iteration through the loop, the > > > "best_delta" variable is uninitialized so re-order the condition to > > > prevent reading uninitialized memory. > > > > > > Fixes: 4aefe1c2bd0c ("iio: imu: add Bosch Sensortec BNO055 core driver") > > > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> > > > > You reported this a while back along with a second issue (false positive) > > with hwval. > > > > I posted a patch fixing both > > https://lore.kernel.org/linux-iio/20221002145324.3776484-1-jic23@kernel.org/ > > > > I don't really care which patch goes in, but curious to reasoning to not also > > deal with the hwval warning here? > > > Meh. Rather than not applying either patch, I'll pick this one up. We can > deal with the hwval warning at a later date if necessary. > Sorry, I am behind in my email. There was a couple weeks in between sending the bug report and the patch so I forgot about it. Also I send those bug reports from a different system so I don't check that as part of the QC process. I'm moving to checking vger instead of grepping my outbox but I haven't automated that yet. I don't get a "hwval" warning on my system so using the cross function DB must have silenced the false positive. regards, dan carpenter
diff --git a/drivers/iio/imu/bno055/bno055.c b/drivers/iio/imu/bno055/bno055.c index 307557a609e3..52744dd98e65 100644 --- a/drivers/iio/imu/bno055/bno055.c +++ b/drivers/iio/imu/bno055/bno055.c @@ -632,7 +632,7 @@ static int bno055_set_regmask(struct bno055_priv *priv, int val, int val2, return -EINVAL; } delta = abs(tbl_val - req_val); - if (delta < best_delta || first) { + if (first || delta < best_delta) { best_delta = delta; hwval = i; first = false;
This bug is basically harmless, although it will trigger a runtime warning if you use KMSan. On the first iteration through the loop, the "best_delta" variable is uninitialized so re-order the condition to prevent reading uninitialized memory. Fixes: 4aefe1c2bd0c ("iio: imu: add Bosch Sensortec BNO055 core driver") Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> --- drivers/iio/imu/bno055/bno055.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)