On Wed, Jan 07, 2015 at 07:21:23PM +0530, Ashay Jaiswal wrote: > The regulator framework maintains a list of consumer regulators > for a regulator device and protects it from concurrent access > using the regulator device's mutex lock. > In the case of regulator_put() the consumer is removed without > holding the regulator device's mutex, resulting in a race condition > between any regulator operation which traverses the consumer list > and regulator_put() which releases the consumer regulator. > Fix this race condition by holding the regulator device's mutex while > removing and releasing the consumer regulator. This is a good spot thanks but I think your analysis here is missing a bit - it's not just the list manipulation that affects the rdev, it's also the reference count in the rdev and the exclusive flag. Indeed some of this issue applies on the _get() side too, while we do add the regulator to the list under the rdev mutex we don't have the mutex when we update the reference count meaning that we've got a potential issue with that. That *is* kind of separate though so could be dealt with in a separate patch. The lock region also seems too wide, the lock is only needed for the operations that affect the rdev not for the operations only on the object being freed - holding the lock for too long means impacting other users and some of the cleanup is potentially expensive. The comment at the top of the function needs updating too, it currently says that the lock is held in the caller but this applies only to the regulator_list_mutex.
Attachment:
signature.asc
Description: Digital signature
