* Re: [PATCH 2/5] pinctrl: rockchip: fix leaked of_node references [not found] <201904121645298604058@zte.com.cn> @ 2019-04-12 11:20 ` Heiko Stübner 0 siblings, 0 replies; 3+ messages in thread From: Heiko Stübner @ 2019-04-12 11:20 UTC (permalink / raw) To: wen.yang99 Cc: linux-kernel, wang.yi59, linus.walleij, linux-gpio, linux-arm-kernel, linux-rockchip 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 <wen.yang99@zte.com.cn> > > > Cc: Linus Walleij <linus.walleij@linaro.org> > > > Cc: Heiko Stuebner <heiko@sntech.de> > > > 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 ^ permalink raw reply [flat|nested] 3+ messages in thread
[parent not found: <1555048943-39589-1-git-send-email-wen.yang99@zte.com.cn>]
* [PATCH 2/5] pinctrl: rockchip: fix leaked of_node references [not found] <1555048943-39589-1-git-send-email-wen.yang99@zte.com.cn> @ 2019-04-12 6:02 ` Wen Yang 2019-04-12 8:08 ` Heiko Stübner 0 siblings, 1 reply; 3+ messages in thread From: Wen Yang @ 2019-04-12 6:02 UTC (permalink / raw) To: linux-kernel Cc: wang.yi59, Heiko Stuebner, linux-rockchip, Linus Walleij, linux-gpio, Wen Yang, linux-arm-kernel 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 <wen.yang99@zte.com.cn> Cc: Linus Walleij <linus.walleij@linaro.org> Cc: Heiko Stuebner <heiko@sntech.de> 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"); -- 2.9.5 ^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH 2/5] pinctrl: rockchip: fix leaked of_node references 2019-04-12 6:02 ` Wen Yang @ 2019-04-12 8:08 ` Heiko Stübner 0 siblings, 0 replies; 3+ messages in thread From: Heiko Stübner @ 2019-04-12 8:08 UTC (permalink / raw) To: Wen Yang Cc: linux-kernel, wang.yi59, Linus Walleij, linux-gpio, linux-arm-kernel, linux-rockchip Hi, Am Freitag, 12. April 2019, 08:02:20 CEST schrieb Wen Yang: > 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 <wen.yang99@zte.com.cn> > Cc: Linus Walleij <linus.walleij@linaro.org> > Cc: Heiko Stuebner <heiko@sntech.de> > 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? Heiko ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2019-04-12 11:20 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <201904121645298604058@zte.com.cn>
2019-04-12 11:20 ` [PATCH 2/5] pinctrl: rockchip: fix leaked of_node references Heiko Stübner
[not found] <1555048943-39589-1-git-send-email-wen.yang99@zte.com.cn>
2019-04-12 6:02 ` Wen Yang
2019-04-12 8:08 ` Heiko Stübner
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).