linux-kernel - Re: [PATCH] regulator: core: Fix enable GPIO reference counting

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:	Mon, 2 Mar 2015 18:47:22 +0000
From:	Mark Brown <broonie@...nel.org>
To:	Doug Anderson <dianders@...omium.org>
Cc:	milo.kim@...com, axel.lin@...ics.com,
	Dmitry Torokhov <dmitry.torokhov@...il.com>, olof@...om.net,
	javier.martinez@...labora.co.uk, Paul Stewart <pstew@...omium.org>,
	stable@...r.kernel.org, lgirdwood@...il.com,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] regulator: core: Fix enable GPIO reference counting

On Fri, Feb 27, 2015 at 11:41:03AM -0800, Doug Anderson wrote:
> It is possible for _regulator_do_enable() to be called for an
> already-enabled rdev, like in regulator_suspend_finish().  If we were
> using an enable pin (rdev->ena_pin is set) then we'd end up
> incrementing the reference count in regulator_ena_gpio_ctrl() over and
> over again without a decrement.  That prevented the GPIO from going to
> the "off" state even after all users were disabled.

> Fix this by avoiding the call to regulator_ena_gpio_ctrl() when it's
> not needed.

There's a big jump in this changelog where you assert that we're
avoiding the call "when it's not needed" without explaining the
situations in which this is the case or why.  

Looking at the code it seems that you're adding checks to skip calls in
the standard enable and disable paths but not touching other paths,
based on this patch by itself I can't tell if this is a good idea or
not.  It certainly doesn't feel robust - if we're missing reference
counting skipping operations seems likely to lead to other bugs popping
up elsewhere when the other user that isn't doing a disable currently
decides to start doing so.

> Signed-off-by: Doug Anderson <dianders@...omium.org>
> Fixes: 967cfb18c0e3 ("regulator: core: manage enable GPIO list")

Fixes normally goes first.

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