Re: [PATCH v3 phy-next 5/8] soc: fsl: guts: make fsl_soc_data available after fsl_guts_init()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] NULL pointer dereference in the error handling path of `fsl_guts_init()`.
--

--- Patch [5]: [PATCH v3 phy-next 5/8] soc: fsl: guts: make fsl_soc_data available after fsl_guts_init() ---
commit d316b69e1c587b56ad6f914ddbd2e412490c8594
Author: Vladimir Oltean <vladimir.oltean@nxp.com>
soc: fsl: guts: make fsl_soc_data available after fsl_guts_init()

This commit preserves the soc_data determined during fsl_guts_init() so it
can be referenced later for RCW override methods. It updates the error
handling paths to properly clean up the saved state if initialization fails.

> diff --git a/drivers/soc/fsl/guts.c b/drivers/soc/fsl/guts.c
> --- a/drivers/soc/fsl/guts.c
> +++ b/drivers/soc/fsl/guts.c
> @@ -231,10 +232,9 @@ static int __init fsl_guts_init(void)
>  {
> -	struct soc_device_attribute *soc_dev_attr;
> +	struct soc_device_attribute *soc_dev_attr = NULL;
>  	static struct soc_device *soc_dev;

[ ... ]

> @@ -243,12 +243,12 @@ static int __init fsl_guts_init(void)
>  	np = of_find_matching_node_and_match(NULL, fsl_guts_of_match, &match);
>  	if (!np)
>  		return 0;
> -	soc_data = match->data;
> +	soc.data = match->data;
>  
>  	soc.dcfg_ccsr = of_iomap(np, DCFG_CCSR);
>  	if (!soc.dcfg_ccsr) {
>  		of_node_put(np);
> -		return -ENOMEM;
> +		goto err_nomem;
>  	}

[Severity: High]
If of_iomap() fails here, the code executes a goto to the error path while
soc_dev_attr is still explicitly NULL. 

Down in the error cleanup path, the code unconditionally dereferences 
soc_dev_attr to retrieve its members:

> @@ -311,8 +311,11 @@ static int __init fsl_guts_init(void)
>  	kfree(soc_dev_attr->revision);
>  	kfree(soc_dev_attr->serial_number);
>  	kfree(soc_dev_attr);

Will this trigger a NULL pointer dereference?

While kfree() itself handles NULL pointers gracefully, evaluating
soc_dev_attr->revision and soc_dev_attr->serial_number requires reading
memory from the NULL soc_dev_attr pointer before passing the argument to
kfree().

> -	iounmap(soc.dcfg_ccsr);
> -	soc.dcfg_ccsr = NULL;
> +	if (soc.dcfg_ccsr) {
> +		iounmap(soc.dcfg_ccsr);
> +		soc.dcfg_ccsr = NULL;
> +	}
> +	soc.data = NULL;
>  
>  	return ret;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260720133642.136324-1-vladimir.oltean@nxp.com?part=5

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy