From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752818AbbGOMqx (ORCPT ); Wed, 15 Jul 2015 08:46:53 -0400 Received: from lists.s-osg.org ([54.187.51.154]:52930 "EHLO lists.s-osg.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751843AbbGOMqa (ORCPT ); Wed, 15 Jul 2015 08:46:30 -0400 Subject: Re: [PATCH] regulator: core: Fix memory leak in regulator_resolve_supply() To: Mark Brown References: <1436883709-9337-1-git-send-email-javier@osg.samsung.com> <55A61C0E.4040707@osg.samsung.com> <20150715112730.GN11162@sirena.org.uk> From: Javier Martinez Canillas X-Enigmail-Draft-Status: N1110 Cc: Krzysztof Kozlowski , Liam Girdwood , linux-kernel@vger.kernel.org Message-ID: <55A65621.4080109@osg.samsung.com> Date: Wed, 15 Jul 2015 14:46:25 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.0.1 MIME-Version: 1.0 In-Reply-To: <20150715112730.GN11162@sirena.org.uk> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hello Mark, On 07/15/2015 01:27 PM, Mark Brown wrote: > On Wed, Jul 15, 2015 at 10:38:38AM +0200, Javier Martinez Canillas wrote: >> On 07/15/2015 10:01 AM, Krzysztof Kozlowski wrote: > >>> The _regulator_put() reverts more work than create_regulator() did, >>> e.g.: module_put and rdev->open_count--. Maybe you need a >>> destroy_regulator() function? > >> Yes, it reverts more work than create_regulator() but the intention is to >> revert what set_supply() did. If you look at the set_supply() function, >> it does supply_rdev->open_count++. > >> I did indeed missed the module_put() but now looking at the code again, I > > Me too, I've dropped the patch. At first glance everything looked safe > for multiple calls. > Ok. >> wonder if the problem is not that set_supply() is missing a try_module_get() >> to be consistent with what the _regulator_get() function does. > > The problem is more that it's a separate implementation and not just > using _regulator_get() I think. A separate, rarely used, path is likely > to have this sort of issue. > Exactly, do you agree then that a try_module_get() is missing in set_supply()? It is OK if I add that in the same patch in v2 or do you prefer that to be in a separate patch? Best regards, -- Javier Martinez Canillas Open Source Group Samsung Research America