linux-hardening - Re: [PATCH v4 000/115] pwm: Fix lifetime issues for pwm

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]

Date: Fri, 8 Dec 2023 16:41:39 +0100
From: Thierry Reding <thierry.reding@...il.com>
To: Uwe Kleine-König <u.kleine-koenig@...gutronix.de>
Cc: kernel@...gutronix.de, linux-pwm@...r.kernel.org,
	Bartosz Golaszewski <brgl@...ev.pl>, linux-leds@...r.kernel.org,
	chrome-platform@...ts.linux.dev,
	linux-arm-kernel@...ts.infradead.org, linux-mips@...r.kernel.org,
	linux-mediatek@...ts.infradead.org,
	linux-amlogic@...ts.infradead.org,
	linux-rpi-kernel@...ts.infradead.org,
	linux-samsung-soc@...r.kernel.org, linux-riscv@...ts.infradead.org,
	linux-stm32@...md-mailman.stormreply.com,
	linux-sunxi@...ts.linux.dev, greybus-dev@...ts.linaro.org,
	linux-staging@...ts.linux.dev, linux-doc@...r.kernel.org,
	asahi@...ts.linux.dev, platform-driver-x86@...r.kernel.org,
	linux-rockchip@...ts.infradead.org, dri-devel@...ts.freedesktop.org,
	linux-hardening@...r.kernel.org
Subject: Re: [PATCH v4 000/115] pwm: Fix lifetime issues for pwm_chips

On Wed, Dec 06, 2023 at 12:43:14PM +0100, Uwe Kleine-König wrote:
> Hello,
> 
> This series is based on Thierry's for-next.
> 
> It starts with some cleanups that were all sent out separately already:
> 
>  - "pwm: Reduce number of pointer dereferences in pwm_device_request()"
>    https://lore.kernel.org/linux-pwm/20231130072105.966848-1-u.kleine-koenig@pengutronix.de
>  - "pwm: crc: Use consistent variable naming for driver data"
>    https://lore.kernel.org/linux-pwm/20231130074133.969806-1-u.kleine-koenig@pengutronix.de
>  - Two leds/qcom-lpg patches
>    https://lore.kernel.org/linux-leds/20231126095230.683204-1-u.kleine-koenig@pengutronix.de
>    Lee already claimed to have taken the series already, but it's not yet in
>    next.
>  - "leds: qcom-lpg: Introduce a wrapper for getting driver data from a pwm chip"
>    https://lore.kernel.org/linux-leds/20231124215208.616551-3-u.kleine-koenig@pengutronix.de
> 
> In the following patches I changed:
> 
>  - "pwm: cros-ec: Change prototype of helper to prepare further changes" +
>    This was simplified in response to feedback by Tzung-Bi Shih
>  - Make pwmchip_priv() static (and don't export it), let drivers use a new
>    pwmchip_get_drvdata() instead.
>  - For drm/ti-sn65dsi86.c and leds/qcom-lpg make use of
>    pwmchip_set_drvdata() which makes the conversion to
>    devm_pwmchip_alloc() much prettier.
>  - Some cleanups here and there
>  - Add review tags received in v3
>    I kept all tags even though the pwmchip_alloc() patches changed
>    slightly. Most of the time that's only
>    s/pwmchip_priv/pwmchip_get_drvdata/ though. Still, if you object,
>    just tell me. (This affects Paul Cercueil on patch #68, Conor Dooley
>    on patch #76 and Greg for patch #109.)
> 
> I kept the pwmchip_alloc() + pwmchip_register() approach despite Bart
> not liking it. To balance that out I don't like Bart's alternative
> approach. There are no technically relevant differences between the two
> approaches and no benchmarks that show either of the two to be better
> than the other. Conceptually the design ideas around pwmchip_alloc() +
> pwmchip_register() are used in several other subsystems, so it's a
> proven way to do things.

[Trimming the recipients, keeping only Bart and the mailing lists.]

I do think there are technically relevant differences. For one, the
better we isolate the internal data structure, the easier this becomes
to manage. I'm attaching a patch that I've prototyped which should
basically get us to somewhere around patch 110 of this series but with
something like 1/8th of the changes. It doesn't need every driver to
change and (mostly) decouples driver code from the core code.

Now, I know that you think this is all bad because it's not a single
allocation, but I much prefer the end result because it's got the driver
and internals much more cleanly separated. Going forward I think it
would be easier to apply all the ref-counting on top of that because we
only need to keep the PWM framework-internal data structure alive after
a PWM chip has gone away.

Thierry

View attachment "0001-pwm-Isolate-internal-data-into-a-separate-structure.patch" of type "text/plain" (31614 bytes)

Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)