Received-SPF: pass (google.com: domain of linux-kernel+bounces-133007-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Date: Fri, 5 Apr 2024 13:41:44 +0100
From: Conor Dooley <conor.dooley@microchip.com>
To: Jonathan Cameron <jic23@kernel.org>
CC: <marius.cristea@microchip.com>, <lars@metafoo.de>, <robh+dt@kernel.org>,
	<jdelvare@suse.com>, <linux@roeck-us.net>, <linux-hwmon@vger.kernel.org>,
	<krzysztof.kozlowski+dt@linaro.org>, <conor+dt@kernel.org>,
	<linux-iio@vger.kernel.org>, <devicetree@vger.kernel.org>,
	<linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v5 2/2] iio: adc: adding support for PAC193x
Message-ID: <20240405-embellish-bonnet-ab5f10560d93@wendy>
References: <20240222164206.65700-1-marius.cristea@microchip.com>
 <20240222164206.65700-3-marius.cristea@microchip.com>
 <20240224191559.40d233db@jic23-huawei>
Precedence: bulk
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha256;
	protocol="application/pgp-signature"; boundary="rftDiodiuYntSj0z"
Content-Disposition: inline
In-Reply-To: <20240224191559.40d233db@jic23-huawei>

--rftDiodiuYntSj0z
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Sat, Feb 24, 2024 at 07:15:59PM +0000, Jonathan Cameron wrote:
> On Thu, 22 Feb 2024 18:42:06 +0200
> <marius.cristea@microchip.com> wrote:
>=20
> > From: Marius Cristea <marius.cristea@microchip.com>
> >=20
> > This is the iio driver for Microchip
> > PAC193X series of Power Monitor with Accumulator chip family.
> >=20
> > Signed-off-by: Marius Cristea <marius.cristea@microchip.com>
> So I had a few comments on this, but nothing that can't be cleaned up lat=
er.
> + I'll fix the thing the bots didn't like on the bindings.
>=20
> 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/ad=
c/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 =3D PAC1934_MAX_VSENSE_RSHIFTED_BY_16B;
		if (chan->scan_type.sign =3D=3D 'u')
			*val2 =3D info->shunts[channel];
		else
			*val2 =3D 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.

--rftDiodiuYntSj0z
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iHUEABYIAB0WIQRh246EGq/8RLhDjO14tDGHoIJi0gUCZg/xiAAKCRB4tDGHoIJi
0txtAP9vdnuMITiNcZxaL8SMlaKjSPQD1+HFCcXJLC7ZFeBlYwD9GJvPd7VFEWXq
582boF44JXcw1Oov6i1opoQf1CMElwY=
=Eo5n
-----END PGP SIGNATURE-----

--rftDiodiuYntSj0z--