linux-kernel - Re: [PATCH v5 1/8] drivers: kunit: Generic helpers for test device creation

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:   Thu, 23 Mar 2023 17:36:39 +0100
From:   Maxime Ripard <maxime@...no.tech>
To:     Matti Vaittinen <mazziesaccount@...il.com>
Cc:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Matti Vaittinen <matti.vaittinen@...rohmeurope.com>,
        "Rafael J. Wysocki" <rafael@...nel.org>,
        Brendan Higgins <brendan.higgins@...ux.dev>,
        David Gow <davidgow@...gle.com>,
        Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
        Heikki Krogerus <heikki.krogerus@...ux.intel.com>,
        linux-kernel@...r.kernel.org, linux-kselftest@...r.kernel.org,
        kunit-dev@...glegroups.com, Stephen Boyd <sboyd@...nel.org>,
        Jonathan Cameron <jic23@...nel.org>, linux-iio@...r.kernel.org
Subject: Re: [PATCH v5 1/8] drivers: kunit: Generic helpers for test device
 creation

On Thu, Mar 23, 2023 at 03:02:03PM +0200, Matti Vaittinen wrote:
> On 3/23/23 14:29, Maxime Ripard wrote:
> > On Thu, Mar 23, 2023 at 02:16:52PM +0200, Matti Vaittinen wrote:
> > 
> > This is the description of what was happening:
> > https://lore.kernel.org/dri-devel/20221117165311.vovrc7usy4efiytl@houat/
>
> Thanks Maxime. Do I read this correcty. The devm_ unwinding not being done
> when root_device_register() is used is not because root_device_unregister()
> would not trigger the unwinding - but rather because DRM code on top of this
> device keeps the refcount increased?

There's a difference of behaviour between a root_device and any device
with a bus: the root_device will only release the devm resources when
it's freed (in device_release), but a bus device will also do it in
device_del (through bus_remove_device() -> device_release_driver() ->
device_release_driver_internal() -> __device_release_driver() ->
device_unbind_cleanup(), which are skipped (in multiple places) if
there's no bus and no driver attached to the device).

It does affect DRM, but I'm pretty sure it will affect any framework
that deals with device hotplugging by deferring the framework structure
until the last (userspace) user closes its file descriptor. So I'd
assume that v4l2 and cec at least are also affected, and most likely
others.

> If this is the case, then it sounds like a DRM specific issue to me.

I mean, I guess. One could also argue that it's because IIO doesn't
properly deal with hotplugging. I'm not sure how that helps. Those are
common helpers which should accommodate every framework, and your second
patch breaks the kunit tests for DRM anyway.

> Whether it is a feature or bug is beyond my knowledge. Still, I would
> not say using the root_device_[un]register() in generic code is not
> feasible - unless all other subsytems have similar refcount handling.
> 
> Sure thing using root_device_register() root_device_unregister() in DRM does
> not work as such. This, however, does not mean the generic kunit helpers
> should use platform_devices to force unwinding?

platform_devices were a quick way to get a device that would have a bus
and a driver bound to fall into the right patch above. We probably
shouldn't use platform_devices and a kunit_device sounds like the best
idea, but the test linked in the original mail I pointed you to should
work with whatever we come up with. It works with multiple (platform,
PCI, USB, etc) buses, so the mock we create should behave like their
real world equivalents.

Maxime

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