| Message ID | f5e260e9-d7a8-4dae-b7ea-f1bbb1760e60@stanley.mountain (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [next] iio: adc: ad4851: Fix signedness bug in ad4851_calibrate() | expand |
On Fri, 2025-02-28 at 12:36 +0300, Dan Carpenter wrote: > The "c" variable is used to store error codes from ad4851_find_opt() > so it has to be signed for the error handling to work. Change it > to type int. > > Fixes: 6250803fe2ec ("iio: adc: ad4851: add ad485x driver") > Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org> > --- Reviewed-by: Nuno Sá <nuno.sa@analog.com> > Btw, in ad4851_write_raw() there are some checks for negative but > the multiplication could still wrap and turn negative so we might want to > add an upper bounds as well: > > drivers/iio/adc/ad4851.c:826 ad4851_write_raw() > warn: potential integer overflow from user 'val * st->osr' > > drivers/iio/adc/ad4851.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/iio/adc/ad4851.c b/drivers/iio/adc/ad4851.c > index 1ad37084355e..98ebc853db79 100644 > --- a/drivers/iio/adc/ad4851.c > +++ b/drivers/iio/adc/ad4851.c > @@ -492,11 +492,11 @@ static int ad4851_find_opt(const unsigned long *field, > unsigned int start, > static int ad4851_calibrate(struct iio_dev *indio_dev) > { > struct ad4851_state *st = iio_priv(indio_dev); > - unsigned int opt_delay, num_lanes, delay, i, s, c; > + unsigned int opt_delay, num_lanes, delay, i, s; > enum iio_backend_interface_type interface_type; > DECLARE_BITMAP(pn_status, AD4851_MAX_LANES * AD4851_MAX_IODELAY); > bool status; > - int ret; > + int c, ret; > > ret = iio_backend_interface_type_get(st->back, &interface_type); > if (ret)
On Fri, 28 Feb 2025 13:24:20 +0000 Nuno Sá <noname.nuno@gmail.com> wrote: > On Fri, 2025-02-28 at 12:36 +0300, Dan Carpenter wrote: > > The "c" variable is used to store error codes from ad4851_find_opt() > > so it has to be signed for the error handling to work. Change it > > to type int. > > > > Fixes: 6250803fe2ec ("iio: adc: ad4851: add ad485x driver") > > Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org> > > --- > > Reviewed-by: Nuno Sá <nuno.sa@analog.com> Julia + 0-day / coccinelle caught this one as well but seeing as you were both first to report and you sent me a patch I'll just apply this one. Applied to the togreg branch of iio.git. Thanks, Jonathan > > > Btw, in ad4851_write_raw() there are some checks for negative but > > the multiplication could still wrap and turn negative so we might want to > > add an upper bounds as well: > > > > drivers/iio/adc/ad4851.c:826 ad4851_write_raw() > > warn: potential integer overflow from user 'val * st->osr' > > > > drivers/iio/adc/ad4851.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/iio/adc/ad4851.c b/drivers/iio/adc/ad4851.c > > index 1ad37084355e..98ebc853db79 100644 > > --- a/drivers/iio/adc/ad4851.c > > +++ b/drivers/iio/adc/ad4851.c > > @@ -492,11 +492,11 @@ static int ad4851_find_opt(const unsigned long *field, > > unsigned int start, > > static int ad4851_calibrate(struct iio_dev *indio_dev) > > { > > struct ad4851_state *st = iio_priv(indio_dev); > > - unsigned int opt_delay, num_lanes, delay, i, s, c; > > + unsigned int opt_delay, num_lanes, delay, i, s; > > enum iio_backend_interface_type interface_type; > > DECLARE_BITMAP(pn_status, AD4851_MAX_LANES * AD4851_MAX_IODELAY); > > bool status; > > - int ret; > > + int c, ret; > > > > ret = iio_backend_interface_type_get(st->back, &interface_type); > > if (ret) > >
diff --git a/drivers/iio/adc/ad4851.c b/drivers/iio/adc/ad4851.c index 1ad37084355e..98ebc853db79 100644 --- a/drivers/iio/adc/ad4851.c +++ b/drivers/iio/adc/ad4851.c @@ -492,11 +492,11 @@ static int ad4851_find_opt(const unsigned long *field, unsigned int start, static int ad4851_calibrate(struct iio_dev *indio_dev) { struct ad4851_state *st = iio_priv(indio_dev); - unsigned int opt_delay, num_lanes, delay, i, s, c; + unsigned int opt_delay, num_lanes, delay, i, s; enum iio_backend_interface_type interface_type; DECLARE_BITMAP(pn_status, AD4851_MAX_LANES * AD4851_MAX_IODELAY); bool status; - int ret; + int c, ret; ret = iio_backend_interface_type_get(st->back, &interface_type); if (ret)
The "c" variable is used to store error codes from ad4851_find_opt() so it has to be signed for the error handling to work. Change it to type int. Fixes: 6250803fe2ec ("iio: adc: ad4851: add ad485x driver") Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org> --- Btw, in ad4851_write_raw() there are some checks for negative but the multiplication could still wrap and turn negative so we might want to add an upper bounds as well: drivers/iio/adc/ad4851.c:826 ad4851_write_raw() warn: potential integer overflow from user 'val * st->osr' drivers/iio/adc/ad4851.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)