linux-kernel - Re: [PATCH] leds: lm3530: fix handling of already enabled regulators

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, 24 Apr 2012 16:48:08 +0800
From:	Axel Lin <axel.lin@...il.com>
To:	"Kim, Milo" <Milo.Kim@...com>
Cc:	Mark Brown <broonie@...nsource.wolfsonmicro.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Shreshtha Kumar Sahu <shreshthakumar.sahu@...ricsson.com>,
	Richard Purdie <rpurdie@...ys.net>,
	Andrew Morton <akpm@...ux-foundation.org>,
	"Girdwood, Liam" <lrg@...com>
Subject: Re: [PATCH] leds: lm3530: fix handling of already enabled regulators

> In my opinion, current source code is the simplest way.
>
>        1. The variable 'drvdata->enabled' is initialized as false.
>        2. The regulator is enabled and the variable is updated when the driver is probed.
>           (If the regulator is already on, it's ok - keep going ...)
>        3. The variable is updated when regulator_enable()/regulator_disable() is called by the lm3530 driver.
>           (If another driver tries to turn off the regulator, it can't be allowed
>            because the reference count was incremented by the lm3530 driver.)

No. Current code is buggy.

Let me put it this way: Assume only this driver is using the regulator.
If the regulator is enabled on boot, which means the reference count
is init to 1.

In probe, the driver assumes the enable variable is false by default and it
calls regulator_enable() in lm3530_init_registers(). This increases
the reference count to 2.

Then if we set brt_val to 0 in lm3530_brightness_set, which will then call
regulator_disable(). In this case, the reference count becomes 1, note that
the reference count is not 0 so the LED may be left on even though the driver
thinks it's off.

Regards,
Axel
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/