Hi Conor,
Thanks for reporting the bug. I have detect it and I'm already
working on a patch for it.
Thanks,
Marius
On Fri, 2024-04-05 at 13:41 +0100, Conor Dooley wrote:
> On Sat, Feb 24, 2024 at 07:15:59PM +0000, Jonathan Cameron wrote:
> > On Thu, 22 Feb 2024 18:42:06 +0200
> > <marius.cristea@xxxxxxxxxxxxx> wrote:
> >
> > > From: Marius Cristea <marius.cristea@xxxxxxxxxxxxx>
> > >
> > > This is the iio driver for Microchip
> > > PAC193X series of Power Monitor with Accumulator chip family.
> > >
> > > Signed-off-by: Marius Cristea <marius.cristea@xxxxxxxxxxxxx>
> > So I had a few comments on this, but nothing that can't be cleaned
> > up later.
> > + I'll fix the thing the bots didn't like on the bindings.
> >
> > Series applied to the togreg branch of iio.git and pushed out
> > as testing for 0-day to take a look at it.
>
> I tested this out on v6.9-rc2 and prompted a backtrace when collectd
> started running:
> ------------[ cut here ]------------
> UBSAN: array-index-out-of-bounds in
> /home/conor/stuff/linux/drivers/iio/adc/pac1934.c:857:25
> index 7 is out of range for type 'u32 [4]'
> CPU: 1 PID: 179 Comm: iiod Not tainted 6.9.0-rc2-dirty #1
> Hardware name: Microchip PolarFire-SoC Icicle Kit (DT)
> Call Trace:
> [<ffffffff80006bba>] dump_backtrace+0x28/0x30
> [<ffffffff80bd67d8>] show_stack+0x38/0x44
> [<ffffffff80be7820>] dump_stack_lvl+0x6e/0x9a
> [<ffffffff80be7864>] dump_stack+0x18/0x20
> [<ffffffff80be1452>] ubsan_epilogue+0x10/0x46
> [<ffffffff80615358>] __ubsan_handle_out_of_bounds+0x6a/0x78
> [<ffffffff80981f3a>] pac1934_read_raw+0x20c/0x34c
> [<ffffffff80977c4c>] iio_read_channel_info+0x5c/0xbe
> [<ffffffff8073516e>] dev_attr_show+0x1c/0x4a
> [<ffffffff80387292>] sysfs_kf_seq_show+0x80/0xcc
> [<ffffffff80385b12>] kernfs_seq_show+0x3c/0x4a
> [<ffffffff8031e3d8>] seq_read_iter+0x136/0x2e4
> [<ffffffff80385cde>] kernfs_fop_read_iter+0x38/0x16a
> [<ffffffff802e904a>] vfs_read+0x1be/0x2ba
> [<ffffffff802e9c48>] ksys_read+0x64/0xd2
> [<ffffffff802e9cd6>] __riscv_sys_read+0x20/0x28
> [<ffffffff80be838a>] do_trap_ecall_u+0xee/0x204
> [<ffffffff80bf57d0>] ret_from_exception+0x0/0x64
> ---[ end trace ]---
>
> The device itself only has 4 channels, but in sysfs there are "fake"
> channels for the average voltages and currents too. UBSAN points at:
> case PAC1934_VSENSE_AVG_4_ADDR:
> *val = PAC1934_MAX_VSENSE_RSHIFTED_BY_16B;
> if (chan->scan_type.sign == 'u')
> *val2 = info->shunts[channel];
> else
> *val2 = info->shunts[channel] >> 1;
> return IIO_VAL_FRACTIONAL;
>
> And info->shunts is only valid for the 4 real channels, so I guess
> the
> averaged channels probably need to do a [channel - 4] or similar. I
> dunno if that relation between averaged channels and their "real"
> counterparts is fixed or if different pac devices need different
> values,
> but on my system here that would work.
>
> I do quite like that UBSAN points out the line in question :)
>
> Cheers,
> Conor.
