Thanks Mark for your review comments. > 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. > I have updated the commit message as per your suggestion. I will try to fix the regulator_get path. > 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. > Yes I too agree with your suggestion, updated the lock region to protect only rdev parameters. Also updated the comment on top of the _regulator_put function to reflect regulator_list_mutex lock is held by caller. I will upload next version of patch incorporating all the review comments. Ashay Jaiswal -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
