From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.0 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI, SIGNED_OFF_BY,SPF_PASS autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 0F0A9C10F14 for ; Fri, 12 Apr 2019 11:20:19 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id D200A20674 for ; Fri, 12 Apr 2019 11:20:18 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="n1FXrRaE" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org D200A20674 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=sntech.de Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+infradead-linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20170209; h=Sender: Content-Transfer-Encoding:Content-Type:Cc:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:MIME-Version:References:In-Reply-To: Message-ID:Date:Subject:To:From:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=OTWwrm4bgFfmhlJYfov14nlct0Ng4CLsNv1g+vyKUlI=; b=n1FXrRaEG0VLQX LeiRyKTYNInLM2N33jducyXyTn6bcJ+rijJDWre5qv0z+X2OKx60TAumhRtB0J4Ippe5ahNH4Qn0w KHEKsMFOCcoo2xinsdueUt/86C7hu346aTg4qR1tSsI76+xXbh6g/3U7gQWmlchQq2uYgH51cqSqt esT0dH6xRIlJHfUaI+oifHNWzq7Srm4jdxkxfzMVEqU2AUdSQy5NvTWxEX6jb7rLrE913Nmouq1Lq 6G2b/SecEZ6oo5TsUZP6S1hCktuHRbi4Yb3XOoIEB7FtnZMoLhubP+Zv8tYcXwlLTDlFCW/tBySKs Dkd4EHnjypefzwryZXWw==; Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.90_1 #2 (Red Hat Linux)) id 1hEuEB-0003yK-NT; Fri, 12 Apr 2019 11:20:15 +0000 Received: from gloria.sntech.de ([185.11.138.130]) by bombadil.infradead.org with esmtps (Exim 4.90_1 #2 (Red Hat Linux)) id 1hEuE9-0003xs-5l; Fri, 12 Apr 2019 11:20:14 +0000 Received: from ip5f5a6320.dynamic.kabel-deutschland.de ([95.90.99.32] helo=diego.localnet) by gloria.sntech.de with esmtpsa (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.89) (envelope-from ) id 1hEuE2-00057O-JI; Fri, 12 Apr 2019 13:20:06 +0200 From: Heiko =?ISO-8859-1?Q?St=FCbner?= To: wen.yang99@zte.com.cn Subject: Re: [PATCH 2/5] pinctrl: rockchip: fix leaked of_node references Date: Fri, 12 Apr 2019 13:20:06 +0200 Message-ID: <1737000.EArop0DIDs@diego> In-Reply-To: <201904121645298604058@zte.com.cn> References: <201904121645298604058@zte.com.cn> MIME-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20190412_042013_365788_51CFC291 X-CRM114-Status: GOOD ( 23.35 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: wang.yi59@zte.com.cn, linux-rockchip@lists.infradead.org, linus.walleij@linaro.org, linux-kernel@vger.kernel.org, linux-gpio@vger.kernel.org, linux-arm-kernel@lists.infradead.org Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+infradead-linux-arm-kernel=archiver.kernel.org@lists.infradead.org Hi, Am Freitag, 12. April 2019, 10:45:29 CEST schrieb wen.yang99@zte.com.cn: > > > The call to of_parse_phandle returns a node pointer with refcount > > > incremented thus it must be explicitly decremented after the last > > > usage. > > > > > > Detected by coccinelle with the following warnings: > > > ./drivers/pinctrl/pinctrl-rockchip.c:3221:2-8: ERROR: missing of_node_put; acquired a node pointer with refcount incremented on line 3196, but without a corresponding object release within this function. > > > ./drivers/pinctrl/pinctrl-rockchip.c:3223:1-7: ERROR: missing of_node_put; acquired a node pointer with refcount incremented on line 3196, but without a corresponding object release within this function. > > > > > > Signed-off-by: Wen Yang > > > Cc: Linus Walleij > > > Cc: Heiko Stuebner > > > Cc: linux-gpio@vger.kernel.org > > > Cc: linux-arm-kernel@lists.infradead.org > > > Cc: linux-rockchip@lists.infradead.org > > > Cc: linux-kernel@vger.kernel.org > > > --- > > > drivers/pinctrl/pinctrl-rockchip.c | 1 + > > > 1 file changed, 1 insertion(+) > > > > > > diff --git a/drivers/pinctrl/pinctrl-rockchip.c b/drivers/pinctrl/pinctrl-rockchip.c > > > index 16bf21b..e22d387 100644 > > > --- a/drivers/pinctrl/pinctrl-rockchip.c > > > +++ b/drivers/pinctrl/pinctrl-rockchip.c > > > @@ -3195,6 +3195,7 @@ static int rockchip_get_bank_data(struct rockchip_pin_bank *bank, > > > > > > node = of_parse_phandle(bank->of_node->parent, > > > "rockchip,pmu", 0); > > > + of_node_put(node); > > > if (!node) { > > > if (of_address_to_resource(bank->of_node, 1, &res)) { > > > dev_err(info->dev, "cannot find IO resource for bank\n"); > > > > > > > hmm, the conditional does still use the node pointer, so the of_node_put > > should probably be below the whole if clause? > > Thank you for your comments. > > There may be two methods to fix this issue here. > Method 1, Add of_node_put after the conditional statement: > > diff --git a/drivers/pinctrl/pinctrl-rockchip.c b/drivers/pinctrl/pinctrl-rockchip.c > index 16bf21b..5f822e6 100644 > --- a/drivers/pinctrl/pinctrl-rockchip.c > +++ b/drivers/pinctrl/pinctrl-rockchip.c > @@ -3198,12 +3198,15 @@ static int rockchip_get_bank_data(struct rockchip_pin_bank *bank, > if (!node) { > if (of_address_to_resource(bank->of_node, 1, &res)) { > dev_err(info->dev, "cannot find IO resource for bank\n"); > + of_node_put(node); > return -ENOENT; > } > > base = devm_ioremap_resource(info->dev, &res); > - if (IS_ERR(base)) > + if (IS_ERR(base)) { > + of_node_put(node); > return PTR_ERR(base); > + } > rockchip_regmap_config.max_register = > resource_size(&res) - 4; > rockchip_regmap_config.name = > @@ -3212,6 +3215,7 @@ static int rockchip_get_bank_data(struct rockchip_pin_bank *bank, > base, > &rockchip_regmap_config); > } > + of_node_put(node); > } > > bank->irq = irq_of_parse_and_map(bank->of_node, 0) > > Method 2, Add of_node_put before conditional statement: > diff --git a/drivers/pinctrl/pinctrl-rockchip.c b/drivers/pinctrl/pinctrl-rockchip.c > index 16bf21b..e22d387 100644 > --- a/drivers/pinctrl/pinctrl-rockchip.c > +++ b/drivers/pinctrl/pinctrl-rockchip.c > @@ -3195,6 +3195,7 @@ static int rockchip_get_bank_data(struct rockchip_pin_bank *bank, > > node = of_parse_phandle(bank->of_node->parent, > "rockchip,pmu", 0); > + of_node_put(node); > if (!node) { > if (of_address_to_resource(bank->of_node, 1, &res)) { > dev_err(info->dev, "cannot find IO resource for bank\n"); > > Since we're just determining whether the node pointer is null, and don't need to dereference the node pointer. > So if we use the Method 2, it might be a little bit simpler. > Thanks. personally I prefer to do it cleanly honoring the rules of using of_nodes. So while your method 2 may make it simpler people possibly editing the code later then need to remember that the node actually is already put when it is checked (or possibly even used in some later patch) Heiko _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel