* [PATCH] iio: adc: imx93: fix a signedness bug in imx93_adc_read_raw()
@ 2023-02-14 15:47 Dan Carpenter
2023-02-15 2:16 ` Bough Chen
0 siblings, 1 reply; 3+ messages in thread
From: Dan Carpenter @ 2023-02-14 15:47 UTC (permalink / raw)
To: Haibo Chen
Cc: Jonathan Cameron, Lars-Peter Clausen, Shawn Guo, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam, NXP Linux Team, linux-iio,
kernel-janitors
The problem is these lines:
ret = vref_uv = regulator_get_voltage(adc->vref);
if (ret < 0)
The "ret" variable is type long and "vref_uv" is u32 so that means
the condition can never be true on a 64bit system. A negative error
code from regulator_get_voltage() would be cast to a high positive
u32 value and then remain a high positive value when cast to a long.
The "ret" variable only ever stores ints so it should be declared as
an int. We can delete the "vref_uv" variable and use "ret" directly.
Fixes: 7d02296ac8b8 ("iio: adc: add imx93 adc support")
Signed-off-by: Dan Carpenter <error27@gmail.com>
---
drivers/iio/adc/imx93_adc.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/iio/adc/imx93_adc.c b/drivers/iio/adc/imx93_adc.c
index d8de8284e13d..8c68f0cd48f2 100644
--- a/drivers/iio/adc/imx93_adc.c
+++ b/drivers/iio/adc/imx93_adc.c
@@ -236,8 +236,7 @@ static int imx93_adc_read_raw(struct iio_dev *indio_dev,
{
struct imx93_adc *adc = iio_priv(indio_dev);
struct device *dev = adc->dev;
- long ret;
- u32 vref_uv;
+ int ret;
switch (mask) {
case IIO_CHAN_INFO_RAW:
@@ -253,10 +252,10 @@ static int imx93_adc_read_raw(struct iio_dev *indio_dev,
return IIO_VAL_INT;
case IIO_CHAN_INFO_SCALE:
- ret = vref_uv = regulator_get_voltage(adc->vref);
+ ret = regulator_get_voltage(adc->vref);
if (ret < 0)
return ret;
- *val = vref_uv / 1000;
+ *val = ret / 1000;
*val2 = 12;
return IIO_VAL_FRACTIONAL_LOG2;
--
2.35.1
^ permalink raw reply related [flat|nested] 3+ messages in thread* RE: [PATCH] iio: adc: imx93: fix a signedness bug in imx93_adc_read_raw()
2023-02-14 15:47 [PATCH] iio: adc: imx93: fix a signedness bug in imx93_adc_read_raw() Dan Carpenter
@ 2023-02-15 2:16 ` Bough Chen
2023-04-10 17:12 ` Jonathan Cameron
0 siblings, 1 reply; 3+ messages in thread
From: Bough Chen @ 2023-02-15 2:16 UTC (permalink / raw)
To: Dan Carpenter
Cc: Jonathan Cameron, Lars-Peter Clausen, Shawn Guo, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam, dl-linux-imx,
linux-iio@vger.kernel.org, kernel-janitors@vger.kernel.org
> -----Original Message-----
> From: Dan Carpenter <error27@gmail.com>
> Sent: 2023年2月14日 23:48
> To: Bough Chen <haibo.chen@nxp.com>
> Cc: Jonathan Cameron <jic23@kernel.org>; Lars-Peter Clausen
> <lars@metafoo.de>; Shawn Guo <shawnguo@kernel.org>; Sascha Hauer
> <s.hauer@pengutronix.de>; Pengutronix Kernel Team
> <kernel@pengutronix.de>; Fabio Estevam <festevam@gmail.com>; dl-linux-imx
> <linux-imx@nxp.com>; linux-iio@vger.kernel.org;
> kernel-janitors@vger.kernel.org
> Subject: [PATCH] iio: adc: imx93: fix a signedness bug in imx93_adc_read_raw()
>
> The problem is these lines:
>
> ret = vref_uv = regulator_get_voltage(adc->vref);
> if (ret < 0)
>
> The "ret" variable is type long and "vref_uv" is u32 so that means the condition
> can never be true on a 64bit system. A negative error code from
> regulator_get_voltage() would be cast to a high positive
> u32 value and then remain a high positive value when cast to a long.
>
> The "ret" variable only ever stores ints so it should be declared as an int. We
> can delete the "vref_uv" variable and use "ret" directly.
Thanks for the catching!
Reviewed-by: Haibo Chen <haibo.chen@nxp.com>
Best Regards
Haibo Chen
>
> Fixes: 7d02296ac8b8 ("iio: adc: add imx93 adc support")
> Signed-off-by: Dan Carpenter <error27@gmail.com>
> ---
> drivers/iio/adc/imx93_adc.c | 7 +++----
> 1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/iio/adc/imx93_adc.c b/drivers/iio/adc/imx93_adc.c index
> d8de8284e13d..8c68f0cd48f2 100644
> --- a/drivers/iio/adc/imx93_adc.c
> +++ b/drivers/iio/adc/imx93_adc.c
> @@ -236,8 +236,7 @@ static int imx93_adc_read_raw(struct iio_dev
> *indio_dev, {
> struct imx93_adc *adc = iio_priv(indio_dev);
> struct device *dev = adc->dev;
> - long ret;
> - u32 vref_uv;
> + int ret;
>
> switch (mask) {
> case IIO_CHAN_INFO_RAW:
> @@ -253,10 +252,10 @@ static int imx93_adc_read_raw(struct iio_dev
> *indio_dev,
> return IIO_VAL_INT;
>
> case IIO_CHAN_INFO_SCALE:
> - ret = vref_uv = regulator_get_voltage(adc->vref);
> + ret = regulator_get_voltage(adc->vref);
> if (ret < 0)
> return ret;
> - *val = vref_uv / 1000;
> + *val = ret / 1000;
> *val2 = 12;
> return IIO_VAL_FRACTIONAL_LOG2;
>
> --
> 2.35.1
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] iio: adc: imx93: fix a signedness bug in imx93_adc_read_raw()
2023-02-15 2:16 ` Bough Chen
@ 2023-04-10 17:12 ` Jonathan Cameron
0 siblings, 0 replies; 3+ messages in thread
From: Jonathan Cameron @ 2023-04-10 17:12 UTC (permalink / raw)
To: Bough Chen
Cc: Dan Carpenter, Lars-Peter Clausen, Shawn Guo, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam, dl-linux-imx,
linux-iio@vger.kernel.org, kernel-janitors@vger.kernel.org
On Wed, 15 Feb 2023 02:16:40 +0000
Bough Chen <haibo.chen@nxp.com> wrote:
> > -----Original Message-----
> > From: Dan Carpenter <error27@gmail.com>
> > Sent: 2023年2月14日 23:48
> > To: Bough Chen <haibo.chen@nxp.com>
> > Cc: Jonathan Cameron <jic23@kernel.org>; Lars-Peter Clausen
> > <lars@metafoo.de>; Shawn Guo <shawnguo@kernel.org>; Sascha Hauer
> > <s.hauer@pengutronix.de>; Pengutronix Kernel Team
> > <kernel@pengutronix.de>; Fabio Estevam <festevam@gmail.com>; dl-linux-imx
> > <linux-imx@nxp.com>; linux-iio@vger.kernel.org;
> > kernel-janitors@vger.kernel.org
> > Subject: [PATCH] iio: adc: imx93: fix a signedness bug in imx93_adc_read_raw()
> >
> > The problem is these lines:
> >
> > ret = vref_uv = regulator_get_voltage(adc->vref);
> > if (ret < 0)
> >
> > The "ret" variable is type long and "vref_uv" is u32 so that means the condition
> > can never be true on a 64bit system. A negative error code from
> > regulator_get_voltage() would be cast to a high positive
> > u32 value and then remain a high positive value when cast to a long.
> >
> > The "ret" variable only ever stores ints so it should be declared as an int. We
> > can delete the "vref_uv" variable and use "ret" directly.
>
> Thanks for the catching!
>
> Reviewed-by: Haibo Chen <haibo.chen@nxp.com>
Applied to the fixes-togreg branch of iio.git.
Thanks
Jonathan
>
> Best Regards
> Haibo Chen
> >
> > Fixes: 7d02296ac8b8 ("iio: adc: add imx93 adc support")
> > Signed-off-by: Dan Carpenter <error27@gmail.com>
> > ---
> > drivers/iio/adc/imx93_adc.c | 7 +++----
> > 1 file changed, 3 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/iio/adc/imx93_adc.c b/drivers/iio/adc/imx93_adc.c index
> > d8de8284e13d..8c68f0cd48f2 100644
> > --- a/drivers/iio/adc/imx93_adc.c
> > +++ b/drivers/iio/adc/imx93_adc.c
> > @@ -236,8 +236,7 @@ static int imx93_adc_read_raw(struct iio_dev
> > *indio_dev, {
> > struct imx93_adc *adc = iio_priv(indio_dev);
> > struct device *dev = adc->dev;
> > - long ret;
> > - u32 vref_uv;
> > + int ret;
> >
> > switch (mask) {
> > case IIO_CHAN_INFO_RAW:
> > @@ -253,10 +252,10 @@ static int imx93_adc_read_raw(struct iio_dev
> > *indio_dev,
> > return IIO_VAL_INT;
> >
> > case IIO_CHAN_INFO_SCALE:
> > - ret = vref_uv = regulator_get_voltage(adc->vref);
> > + ret = regulator_get_voltage(adc->vref);
> > if (ret < 0)
> > return ret;
> > - *val = vref_uv / 1000;
> > + *val = ret / 1000;
> > *val2 = 12;
> > return IIO_VAL_FRACTIONAL_LOG2;
> >
> > --
> > 2.35.1
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2023-04-10 16:57 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-02-14 15:47 [PATCH] iio: adc: imx93: fix a signedness bug in imx93_adc_read_raw() Dan Carpenter
2023-02-15 2:16 ` Bough Chen
2023-04-10 17:12 ` Jonathan Cameron
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox