Re: [PATCH] pinctrl: sophgo: fix double free in cv1800_pctrl_dt_node_to_map()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Le 10/10/2024 à 20:33, Dan Carpenter a écrit :
On Thu, Oct 10, 2024 at 07:17:19PM +0200, Christophe JAILLET wrote:
Le 10/10/2024 à 13:18, Harshit Mogalapalli a écrit :
'map' is allocated using devm_* which takes care of freeing the allocated
data, but in error paths there is a call to pinctrl_utils_free_map()
which also does kfree(map) which leads to a double free.

Use kcalloc() instead of devm_kcalloc() as freeing is manually handled.

Fixes: a29d8e93e710 ("pinctrl: sophgo: add support for CV1800B SoC")
Signed-off-by: Harshit Mogalapalli <harshit.m.mogalapalli@xxxxxxxxxx>
---
This is based on static analysis with smatch, only compile tested.
---
   drivers/pinctrl/sophgo/pinctrl-cv18xx.c | 2 +-
   1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pinctrl/sophgo/pinctrl-cv18xx.c b/drivers/pinctrl/sophgo/pinctrl-cv18xx.c
index d18fc5aa84f7..57f2674e75d6 100644
--- a/drivers/pinctrl/sophgo/pinctrl-cv18xx.c
+++ b/drivers/pinctrl/sophgo/pinctrl-cv18xx.c
@@ -221,7 +221,7 @@ static int cv1800_pctrl_dt_node_to_map(struct pinctrl_dev *pctldev,
   	if (!grpnames)
   		return -ENOMEM;
-	map = devm_kcalloc(dev, ngroups * 2, sizeof(*map), GFP_KERNEL);
+	map = kcalloc(ngroups * 2, sizeof(*map), GFP_KERNEL);
   	if (!map)
   		return -ENOMEM;

Hi,

drivers/pinctrl/nuvoton/pinctrl-ma35.c seems to have the same issue.


Yep.  That one is too complicated for Smatch to find.  In this case, the kfree()
happened in the cleanup so it was in the same function.

regards,
dan carpenter




For the records, I spotted the other case with coccinelle:

@ok1@
identifier X, fct;
@@

struct pinctrl_ops X = {
	.dt_node_to_map = fct,
	.dt_free_map = pinconf_generic_dt_free_map,
};

@test1 depends on ok1@
identifier fct = ok1.fct;
identifier fct2 =~ "devm";
expression x =~ "map";
@@

int fct(...)
{
	...
*	x = fct2(...);
	...
}




@ok2@
identifier X, fct;
@@

struct pinctrl_ops X = {
	.dt_node_to_map = fct,
	.dt_free_map = pinctrl_utils_free_map,
};

@test2 depends on ok2@
identifier fct = ok2.fct;
identifier fct2 =~ "devm";
expression x =~ "map";
@@

int fct(...)
{
	...
*	x = fct2(...);
	...
}

CJ




[Index of Archives]     [Kernel Development]     [Kernel Announce]     [Kernel Newbies]     [Linux Networking Development]     [Share Photos]     [IDE]     [Security]     [Git]     [Netfilter]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Device Mapper]

  Powered by Linux