linux-kernel - Re: [PATCH] spi: ensure timely release of driver-allocated resources

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:   Tue, 23 Mar 2021 17:36:06 +0000
From:   Mark Brown <broonie@...nel.org>
To:     Dmitry Torokhov <dmitry.torokhov@...il.com>
Cc:     linux-spi@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] spi: ensure timely release of driver-allocated resources

On Mon, Mar 22, 2021 at 12:38:15PM -0700, Dmitry Torokhov wrote:
> On Mon, Mar 22, 2021 at 12:37:07PM +0000, Mark Brown wrote:

> > This feels like it might make sense to push up to the driver core level
> > then rather than doing in individual buses?

> That is exactly the issue: we can't. Driver core already releases all
> resources when a device is being unbound but that happens after bus
> "remove" code is executed and therefore is too late. The device might
> already be powered down, but various devm release() callbacks will be
> trying to access it.

Can you provide a concrete example of something that is causing problems
here?  If something is trying to access the device after remove() has
run that sounds like it's abusing devres somehow.  It sounded from your
commit log like this was something to do with the amount of time it took
the driver core to action the frees rather than an ordering issue.

> devm only works when you do not mix manual resources with managed ones,
> and when bus code allocates resources themselves (attaching a device to
> a power domain can be viewed as resource acquisition) we violate this
> principle. We could, of course, to make SPI bus' probe() use
> devm_add_action_or_reset() to work in removal of the device from the
> power domain into the stream of devm resources, but that still requires
> changes at bus code, and I believe will complicate matters if we need to
> extend SPI bus code to allocate more resources in probe(). So I opted
> for opening a devm group to separate resources allocated before and
> after probe() to be able to release them in the right order.

Sure, these are standard issues that people create with excessive use of
devm but the device's remove() callback is surely already a concern by
itself here?

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