Re: [PATCH v3 phy-next 3/8] soc: fsl: guts: add a central fsl_guts_read() function

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Dereferencing NULL pointer 'soc_dev_attr' on the error path if its memory allocation fails.
--

--- Patch [3]: [PATCH v3 phy-next 3/8] soc: fsl: guts: add a central fsl_guts_read() function ---
commit cc1312c02ab5c2cd3fed5e2907608e44453c7816
Author: Ioana Ciornei <ioana.ciornei@nxp.com>

soc: fsl: guts: add a central fsl_guts_read() function

This commit adds a central fsl_guts_read() function that handles reading
DCFG registers based on the already determined endianness. This avoids
duplicating the if-else endianness checks across the driver.

> diff --git a/drivers/soc/fsl/guts.c b/drivers/soc/fsl/guts.c
> index a2b4c477b064b..e1b3a054e681b 100644
> --- a/drivers/soc/fsl/guts.c
> +++ b/drivers/soc/fsl/guts.c
> @@ -111,6 +111,14 @@ static struct fsl_soc_guts {
>  	bool little_endian;
>  } soc;
>  
> +static unsigned int fsl_guts_read(const void __iomem *reg)
> +{
> +	if (soc.little_endian)
> +		return ioread32(reg);
> +
> +	return ioread32be(reg);
> +}
> +
>  static const struct fsl_soc_die_attr *fsl_soc_die_match(
>  	u32 svr, const struct fsl_soc_die_attr *matches)
>  {

[ ... ]

> @@ -209,10 +217,7 @@ static int __init fsl_guts_init(void)
>  	}
>  
>  	soc.little_endian = of_property_read_bool(np, "little-endian");
> -	if (soc.little_endian)
> -		svr = ioread32(&soc.dcfg_ccsr->svr);
> -	else
> -		svr = ioread32be(&soc.dcfg_ccsr->svr);
> +	svr = fsl_guts_read(&soc.dcfg_ccsr->svr);
>  	of_node_put(np);
>  
>  	/* Register soc device */

[Severity: High]
Does this code trigger a NULL pointer dereference on the error path if memory
allocation for soc_dev_attr fails?

In fsl_guts_init(), if the kzalloc_obj() allocation for soc_dev_attr fails
just below this block, the code jumps to err_nomem and falls through to the
err label:

fsl_guts_init() {
    ...
	soc_dev_attr = kzalloc_obj(*soc_dev_attr);
	if (!soc_dev_attr)
		goto err_nomem;
    ...
err_nomem:
	ret = -ENOMEM;
err:
	kfree(soc_dev_attr->family);
	kfree(soc_dev_attr->soc_id);
    ...
}

Since soc_dev_attr is NULL when taking this path, won't calling
kfree(soc_dev_attr->family) dereference the NULL pointer during device
initialization?

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