* Re: [PATCH 2/2] staging: iio: light: tsl2583.c: obsolete use of strict_strtoul
[not found] ` <1323700503-30053-2-git-send-email-Johannes.Tenschert@informatik.stud.uni-erlangen.de>
@ 2011-12-12 18:55 ` Dan Carpenter
0 siblings, 0 replies; only message in thread
From: Dan Carpenter @ 2011-12-12 18:55 UTC (permalink / raw)
To: Johannes Tenschert
Cc: devel, gregkh, Jon Brenner, Jonathan Cameron, linux-iio
[-- Attachment #1: Type: text/plain, Size: 2278 bytes --]
These patches need to be CC'd to the iio mailing list, and Jonathan
Cameron as well. Use the ./scripts/get_maintainer.pl script for
hints who should be CC'd.
On Mon, Dec 12, 2011 at 03:35:03PM +0100, Johannes Tenschert wrote:
> @@ -621,7 +621,7 @@ static ssize_t taos_als_trim_store(struct device *dev,
> struct tsl2583_chip *chip = iio_priv(indio_dev);
> unsigned long value;
>
> - if (strict_strtoul(buf, 0, &value))
> + if (kstrtoul(buf, 0, &value))
> return -EINVAL;
>
> if (value)
We save value to
chip->taos_settings.als_gain_trim = value;
als_gain_trim is an int, so on a 64bit system this would truncate
the high bits away and we could get a zero where we don't expect
one.
I don't think it's a problem, but looking at how als_gain_trim is
used, I noticed this potential bug:
362 lux_val = taos_get_lux(indio_dev);
^^^^^^^
lux_val can be zero under certain circumstances.
363 if (lux_val < 0) {
364 dev_err(&chip->client->dev, "taos_als_calibrate failed to get lux\n");
365 return lux_val;
366 }
367 gain_trim_val = (unsigned int) (((chip->taos_settings.als_cal_target)
368 * chip->taos_settings.als_gain_trim) / lux_val);
^^^^^^^^^
leading to a divide by zero bug.
369
370 if ((gain_trim_val < 250) || (gain_trim_val > 4000)) {
371 dev_err(&chip->client->dev,
372 "taos_als_calibrate failed: trim_val of %d is out of range\n",
373 gain_trim_val);
374 return -ENODATA;
375 }
376 chip->taos_settings.als_gain_trim = (int) gain_trim_val;
Also I don't understand why als_gain_trim isn't unsigned. So
basically I think we should make it unsigned and I think we should
use kstrtouint() here. Also instead of returning -EINVAL, we
should preserve the return code from kstrtoul() and return that.
But I don't know the code that well so I may have missed something.
The other divide by zero bug should be fixed in a different patch.
regards,
dan carpenter
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] only message in thread