linux-kernel - Re: [PATCH] regulator: core: Unset supplies when regulators are unregistered

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] [day] [month] [year] [list]

Message-ID: <20170109164039.4hsiqpvx2khriktw@sirena.org.uk>
Date:   Mon, 9 Jan 2017 16:40:39 +0000
From:   Mark Brown <broonie@...nel.org>
To:     Jon Hunter <jonathanh@...dia.com>
Cc:     Liam Girdwood <lgirdwood@...il.com>, linux-kernel@...r.kernel.org,
        linux-tegra@...r.kernel.org,
        Javier Martinez Canillas <javier@...hile0.org>
Subject: Re: [PATCH] regulator: core: Unset supplies when regulators are
 unregistered

On Mon, Jan 09, 2017 at 01:37:18PM +0000, Jon Hunter wrote:
> On 06/01/17 18:29, Mark Brown wrote:

> > Why is a parent device doing this?  This doesn't seem like safe or
> > helpful behaviour and with probe deferral we'd generally expect the
> > device to acquire resources before it starts making use of them.

> It is not really the parent that is doing this. The child regulator in
> this case is physically a separate regulator from the main PMIC device
> that is registering the supply regulators.

The parent of the regulator being unregistered.

> > We can't completely stop people doing this but we do make fairly strong
> > efforts to stop people pulling in use devices.

> Right, but before removing/unregistering a regulator today, we never
> check to see if it is the supply for any other regulator.

We're trying to not specifically look for that by making supplies look
like regular consumers which we were handling by holding a reference on
the module (far from bulletproof but hey).  Special casing them was way
too much of a source of bugs to be sustainable.

> > This seems like storing up trouble for the future, we'll end up with
> > live child devices with parents that weren't around or being refcounted
> > through some of the lifetime of the device which will doubtless come
> > back and bite us later.

> Yes I am not completely happy. Maybe we should wait for the parent
> device to be bound before allowing any of its's regulators to be added
> as supplies for other regulators? I gave the following a quick test and
> this seems to work too ...

Yeah, that thought occurred to me too.  

> The above prevents anyone from using a regulator as a supply if the
> parent has not been bound yet. Therefore, if one of the parent's
> regulators fails to register, after some have already been successfully
> registered, there is no chance any of the successfully added regulators
> will have been already added as a supply to some other regulator in the
> system. Hope that's clear!

Can you cook it up into a proper patch please?  I *think* that should be
OK from a correctness point of view and it should be way more robust (as
well as simpler to implement).

> > Please don't mix different changes into one commit, as covered in
> > SubmittingPatches please send one patch per change.  This makes things
> > easier to 

> Sorry, may be I worded this badly, however, it is a consequence of this
> particular change and otherwise it would not be needed.

Ah, OK.  That makes more sense - at a thousand foot view it wasn't
obvious.

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