linux-kernel - Re: [PATCH 0/3] iio: cleanup masklength usage

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <2630ef8c8363b4fa772a2ff2c95cf115ad3c509d.camel@gmail.com>
Date: Fri, 26 Apr 2024 09:13:13 +0200
From: Nuno Sá <noname.nuno@...il.com>
To: David Lechner <dlechner@...libre.com>, Jonathan Cameron
 <jic23@...nel.org>
Cc: Lars-Peter Clausen <lars@...afoo.de>, Michael Hennerich
 <Michael.Hennerich@...log.com>, Shawn Guo <shawnguo@...nel.org>, Sascha
 Hauer <s.hauer@...gutronix.de>, Pengutronix Kernel Team
 <kernel@...gutronix.de>,  Fabio Estevam <festevam@...il.com>,
 linux-iio@...r.kernel.org, linux-kernel@...r.kernel.org, 
 imx@...ts.linux.dev, linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH 0/3] iio: cleanup masklength usage

On Thu, 2024-04-25 at 10:03 -0500, David Lechner wrote:
> While working on other patches I noticed that a few drivers are setting
> the masklength field of struct iio_dev even though it is marked as
> [INTERN]. It looks like maybe this was not always the case, but we can
> safely clean it up now without breaking anything.
> 
> ---
> David Lechner (3):
>       iio: adc: ad7266: don't set masklength
>       iio: adc: mxs-lradc-adc: don't set masklength
>       iio: buffer: initialize masklength accumulator to 0
> 
>  drivers/iio/adc/ad7266.c          | 1 -
>  drivers/iio/adc/mxs-lradc-adc.c   | 1 -
>  drivers/iio/industrialio-buffer.c | 2 +-
>  3 files changed, 1 insertion(+), 3 deletions(-)
> ---
> base-commit: b80ad8e3cd2712b78b98804d1f59199680d8ed91
> change-id: 20240425-b4-iio-masklength-cleanup-86b632b19901
> 

Hi David,

Nice cleanup. The patches look good to me but there's one thing missing :). As you
correctly noted, the field should be internal to the IIO core and drivers should not
touch it. Hence, you need to make sure not driver is using it so we can move it into
struct iio_dev_opaque [1]. That's the place all the intern fields should, eventually,
end up.

Now, quite some drivers in the trigger handler will read the masklength for looping
with for_each_set_bit(). Hence, the straight thing would be an helper to get it.
Maybe there's a clever way...

I know this is more work than what you had in mind but I think it should be fairly
simple (hopefully) and since you started it :), maybe we can get the whole thing done
and remove another [INTERN] member from the iio_dev struct.

[1]: https://elixir.bootlin.com/linux/latest/source/include/linux/iio/iio-opaque.h#L42

- Nuno Sá