Re: [PATCH v2 0/2] Sanity-check available_scan_masks array

From: Jonathan Cameron
Date: Sat Oct 28 2023 - 12:32:45 EST


On Sat, 21 Oct 2023 19:03:15 +0300
Matti Vaittinen <mazziesaccount@xxxxxxxxx> wrote:

> On 10/21/23 18:55, Jonathan Cameron wrote:
> > On Mon, 16 Oct 2023 14:04:11 +0300
> > Matti Vaittinen <mazziesaccount@xxxxxxxxx> wrote:
> >
> >> Sanity-check available_scan_masks array
> >>
> >> The available_scan_masks is an array of bitmasks representing the
> >> channels which can be simultaneously(*) scanned by a driver from the
> >> device. Following special characteristics apply:
> >>
> >> - When IIO is scanning through the array it will use the first mask
> >> which can be used to scan all enabled channels. This means drivers
> >> should order the array in the order of the preference. This does also
> >> mean that a mask which is a subset of a mask located earler in array
> >> will never be used because the earlier one will be picked by the core.
> >> - Masks wider than size of long are supported only to some extent. The
> >> code scanning through the array will interpret the first mask with
> >> first long zeroed as end-of-array terminator. Changing this behaviour
> >> would make mask-arrays for multi-long masks to be terminated by more
> >> than one zero long. Failure to do so would result kernel to read
> >> beyond the array generating a potentially hazardous bug.
> >>
> >> Add a sanity-check to IIO-device registration emitting a warning if
> >> available_scan_mask array is misordered or if mask width is larger than
> >> a long while available_scan_mask-array is populated. Currently there
> >> should be no in-tree drivers with available_scan_mask populated and mask
> >> wider than a long.
> >>
> >> Revision history:
> >> v1 => v2:
> >> - Add patch 2/2 documenting why iio_scan_mask_match() checks only
> >> a long worth of bits while searching for the end of the
> >> available_scan_mask-array.
> >> - Styling of patch 1/2 as per comments from Jonathan
> >> v1 and related discussion here:
> >> https://lore.kernel.org/lkml/ZRvjuZaQWdZw1U1I@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx/
> >>
> >> Matti Vaittinen (2):
> >> iio: sanity check available_scan_masks array
> >> iio: buffer: document known issue
> >>
> >> drivers/iio/industrialio-buffer.c | 16 ++++++++
> >> drivers/iio/industrialio-core.c | 63 +++++++++++++++++++++++++++++++
> >> 2 files changed, 79 insertions(+)
> >>
> >
> > Hi Matti,
> >
> > Just a quick note to say this looks fine to me, but I don't want to queue it up
> > just yet given proximity to merge window etc.
>
> Makes perfect sense to me.
>
> > I'll aim to pick it up early
> > in next cycle. Give me a poke if I still haven't by rc3 or so.
>
> Ouch... My memory gets worse year by year - well, I'll try to remember :)

I've started queuing stuff up for rebasing post merge window, so I've
added this as well. For now will only be exposed as the testing branch
that 0-day pokes at.

Thanks,

Jonathan

>
> Yours,
> -- Matti
>