On 11/4/22 6:28 AM, Wei Yongjun wrote:
From: Wei Yongjun <weiyongjun1@xxxxxxxxxx>
KASAN report out-of-bounds read as follows:
BUG: KASAN: global-out-of-bounds in afe4403_read_raw+0x42e/0x4c0 [afe4403]
Read of size 4 at addr ffffffffc02ac638 by task cat/279
CPU: 2 PID: 279 Comm: cat Tainted: G N
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.15.0-1 04/01/2014
Call Trace:
<TASK>
afe4403_read_raw+0x42e/0x4c0 [afe4403 141d77410f5466ef049ee2376f5136b77a168de0]
iio_read_channel_info+0x249/0x2e0 [industrialio d0627df60a92bbb9630e68c3e2f98d20dac889ef]
dev_attr_show+0x4b/0xa0 drivers/base/core.c:2195
sysfs_kf_seq_show+0x1ec/0x390 fs/sysfs/file.c:59
seq_read_iter+0x48d/0x10b0 fs/seq_file.c:230
kernfs_fop_read_iter+0x4e6/0x710 fs/kernfs/file.c:275
call_read_iter include/linux/fs.h:2153 [inline]
new_sync_read fs/read_write.c:389 [inline]
vfs_read+0x5f2/0x890 fs/read_write.c:470
ksys_read+0x106/0x220 fs/read_write.c:613
do_syscall_x64 arch/x86/entry/common.c:50 [inline]
do_syscall_64+0x38/0xa0 arch/x86/entry/common.c:80
entry_SYSCALL_64_after_hwframe+0x72/0xdc
RIP: 0033:0x7fd8611cf992
</TASK>
The buggy address belongs to the variable:
afe4403_channel_leds+0x18/0xffffffffffffe9e0 [afe4403]
This issue can be reproduced by singe command:
$ cat /sys/bus/spi/devices/spi0.0/iio\:device0/in_intensity6_raw
The array size of afe4403_channel_leds is less than channels, so access
with chan->address cause OOB read in afe4403_read_raw. Fix it by moving
access before use it.
Fixes: b36e8257641a ("iio: health/afe440x: Use regmap fields")
Signed-off-by: Wei Yongjun <weiyongjun1@xxxxxxxxxx>
---
drivers/iio/health/afe4403.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/iio/health/afe4403.c b/drivers/iio/health/afe4403.c
index 3bb4028c5d74..14213a48e349 100644
--- a/drivers/iio/health/afe4403.c
+++ b/drivers/iio/health/afe4403.c
@@ -246,7 +246,7 @@ static int afe4403_read_raw(struct iio_dev *indio_dev,
{
struct afe4403_data *afe = iio_priv(indio_dev);
unsigned int reg = afe4403_channel_values[chan->address];
Good find and the fix does look valid, but can we also move this
access for 'reg' to right before we use it also? Just for consistency.
Same for the 'afe4403_channel_leds' access below in afe4403_write_raw(),
and same for value_reg in your patch for afe4404. Then both patches can have:
Acked-by: Andrew Davis <afd@xxxxxx>
- unsigned int field = afe4403_channel_leds[chan->address];
+ unsigned int field;
int ret;
switch (chan->type) {
@@ -262,6 +262,7 @@ static int afe4403_read_raw(struct iio_dev *indio_dev,
case IIO_CURRENT:
switch (mask) {
case IIO_CHAN_INFO_RAW:
+ field = afe4403_channel_leds[chan->address];
ret = regmap_field_read(afe->fields[field], val);
if (ret)
return ret;
