linux-kernel - RE: [PATCH 01/21] pinctrl: ti: iodelay: Use scope based of_node

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Thu, 2 May 2024 00:28:42 +0000
From: Peng Fan <peng.fan@....com>
To: Dan Carpenter <dan.carpenter@...aro.org>, "Peng Fan (OSS)"
	<peng.fan@....nxp.com>
CC: Linus Walleij <linus.walleij@...aro.org>, Thierry Reding
	<thierry.reding@...il.com>, Jonathan Hunter <jonathanh@...dia.com>, Dvorkin
 Dmitry <dvorkin@...bo.com>, Wells Lu <wellslutw@...il.com>, Maxime Coquelin
	<mcoquelin.stm32@...il.com>, Alexandre Torgue <alexandre.torgue@...s.st.com>,
	Emil Renner Berthing <kernel@...il.dk>, Jianlong Huang
	<jianlong.huang@...rfivetech.com>, Hal Feng <hal.feng@...rfivetech.com>,
	Orson Zhai <orsonzhai@...il.com>, Baolin Wang
	<baolin.wang@...ux.alibaba.com>, Chunyan Zhang <zhang.lyra@...il.com>, Viresh
 Kumar <vireshk@...nel.org>, Shiraz Hashim <shiraz.linux.kernel@...il.com>,
	"soc@...nel.org" <soc@...nel.org>, Krzysztof Kozlowski <krzk@...nel.org>,
	Sylwester Nawrocki <s.nawrocki@...sung.com>, Alim Akhtar
	<alim.akhtar@...sung.com>, Geert Uytterhoeven <geert+renesas@...der.be>,
	Patrice Chotard <patrice.chotard@...s.st.com>, Heiko Stuebner
	<heiko@...ech.de>, Damien Le Moal <dlemoal@...nel.org>, Ludovic Desroches
	<ludovic.desroches@...rochip.com>, Nicolas Ferre
	<nicolas.ferre@...rochip.com>, Alexandre Belloni
	<alexandre.belloni@...tlin.com>, Claudiu Beznea <claudiu.beznea@...on.dev>,
	Aisheng Dong <aisheng.dong@....com>, Fabio Estevam <festevam@...il.com>,
	Shawn Guo <shawnguo@...nel.org>, Jacky Bai <ping.bai@....com>, Pengutronix
 Kernel Team <kernel@...gutronix.de>, Chester Lin <chester62515@...il.com>,
	Matthias Brugger <mbrugger@...e.com>, "Ghennadi Procopciuc (OSS)"
	<ghennadi.procopciuc@....nxp.com>, Sean Wang <sean.wang@...nel.org>, Matthias
 Brugger <matthias.bgg@...il.com>, AngeloGioacchino Del Regno
	<angelogioacchino.delregno@...labora.com>, Sascha Hauer
	<s.hauer@...gutronix.de>, Andrew Jeffery <andrew@...econstruct.com.au>, Joel
 Stanley <joel@....id.au>, "linux-gpio@...r.kernel.org"
	<linux-gpio@...r.kernel.org>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>, "linux-tegra@...r.kernel.org"
	<linux-tegra@...r.kernel.org>, "linux-arm-kernel@...ts.infradead.org"
	<linux-arm-kernel@...ts.infradead.org>,
	"linux-stm32@...md-mailman.stormreply.com"
	<linux-stm32@...md-mailman.stormreply.com>,
	"linux-samsung-soc@...r.kernel.org" <linux-samsung-soc@...r.kernel.org>,
	"linux-renesas-soc@...r.kernel.org" <linux-renesas-soc@...r.kernel.org>,
	"linux-rockchip@...ts.infradead.org" <linux-rockchip@...ts.infradead.org>,
	"linux-riscv@...ts.infradead.org" <linux-riscv@...ts.infradead.org>,
	"linux-mediatek@...ts.infradead.org" <linux-mediatek@...ts.infradead.org>,
	"imx@...ts.linux.dev" <imx@...ts.linux.dev>, "linux-aspeed@...ts.ozlabs.org"
	<linux-aspeed@...ts.ozlabs.org>, "openbmc@...ts.ozlabs.org"
	<openbmc@...ts.ozlabs.org>
Subject: RE: [PATCH 01/21] pinctrl: ti: iodelay: Use scope based of_node_put()
 cleanups

> Subject: Re: [PATCH 01/21] pinctrl: ti: iodelay: Use scope based of_node_put()
> cleanups
> 
> On Wed, May 01, 2024 at 08:55:59PM +0800, Peng Fan (OSS) wrote:
> > @@ -879,16 +874,12 @@ static int ti_iodelay_probe(struct
> platform_device *pdev)
> >  	ret = pinctrl_register_and_init(&iod->desc, dev, iod, &iod->pctl);
> >  	if (ret) {
> >  		dev_err(dev, "Failed to register pinctrl\n");
> > -		goto exit_out;
> > +		return ret;
> >  	}
> >
> >  	platform_set_drvdata(pdev, iod);
> >
> >  	return pinctrl_enable(iod->pctl);
> > -
> > -exit_out:
> > -	of_node_put(np);
> > -	return ret;
> >  }
> 
> This will call of_node_put() on the success path so it's a behavior change.  The
> original code is buggy, it's supposed to call of_node_put() on the success path
> here or in ti_iodelay_remove().
> 
> If it's supposed to call of_node_put() here, then fine, this is bugfix but if it's
> supposed to call it in ti_iodelay_remove() then we need to save the pointer
> somewhere using no_free_ptr().  Probably saving ->np is the safest choice?
> 
> The original code is already a little bit buggy because it doesn't check for
> pinctrl_enable() errors and cleanup.

It was introduced by 
commit 6118714275f0a313ecc296a87ed1af32d9691bed (tag: pinctrl-v4.11-4)
Author: Tony Lindgren <tony@...mide.com>
Date:   Thu Mar 30 09:16:39 2017 -0700

    pinctrl: core: Fix pinctrl_register_and_init() with pinctrl_enable()

of_node_put is expected in probe, not in remove.

Thanks,
Peng.

> 
> 
> diff --git a/drivers/pinctrl/ti/pinctrl-ti-iodelay.c b/drivers/pinctrl/ti/pinctrl-ti-
> iodelay.c
> index 040f2c46a868..f40a1476e4ff 100644
> --- a/drivers/pinctrl/ti/pinctrl-ti-iodelay.c
> +++ b/drivers/pinctrl/ti/pinctrl-ti-iodelay.c
> @@ -156,6 +156,7 @@ struct ti_iodelay_device {
> 
>  	const struct ti_iodelay_reg_data *reg_data;
>  	struct ti_iodelay_reg_values reg_init_conf_values;
> +	struct device_node *np;
>  };
> 
>  /**
> @@ -884,7 +885,12 @@ static int ti_iodelay_probe(struct platform_device
> *pdev)
> 
>  	platform_set_drvdata(pdev, iod);
> 
> -	return pinctrl_enable(iod->pctl);
> +	ret = pinctrl_enable(iod->pctl);
> +	if (ret)
> +		goto exit_out;
> +
> +	iod->np = no_free_ptr(np);
> +	return 0;
> 
>  exit_out:
>  	of_node_put(np);
> @@ -903,6 +909,7 @@ static void ti_iodelay_remove(struct platform_device
> *pdev)
>  		pinctrl_unregister(iod->pctl);
> 
>  	ti_iodelay_pinconf_deinit_dev(iod);
> +	of_node_put(iod->np);
> 
>  	/* Expect other allocations to be freed by devm */  }
> 
> 
>