Re: [PATCH] mtd: spi-nor: sfdp: reduce stack usage in spi_nor_parse_sfdp()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] cleanup: mixing goto-based cleanup and __free() in spi_nor_parse_sfdp()
--

commit 487c324880f913694192ab54591472cbb5bb22c0
Author: Michael Walle <mwalle@kernel.org>

mtd: spi-nor: sfdp: reduce stack usage in spi_nor_parse_sfdp()

This commit allocates the rollback copies of spi_nor_flash_parameter
structures on the heap to reduce stack usage in spi_nor_parse_sfdp().

> diff --git a/drivers/mtd/spi-nor/sfdp.c b/drivers/mtd/spi-nor/sfdp.c
> index beb0f5c66212c..e30cdb5b18ede 100644
> --- a/drivers/mtd/spi-nor/sfdp.c
> +++ b/drivers/mtd/spi-nor/sfdp.c
> @@ -1535,9 +1535,9 @@ int spi_nor_check_sfdp_signature(struct spi_nor *nor)
>   */
>  int spi_nor_parse_sfdp(struct spi_nor *nor)
>  {
> +	struct spi_nor_flash_parameter *save_params __free(kfree) = kmalloc_objs(*save_params, 2);
>  	const struct sfdp_parameter_header *param_header, *bfpt_header;

[ ... ]

> @@ -1724,7 +1726,7 @@ int spi_nor_parse_sfdp(struct spi_nor *nor)
>  free_param_headers:
>  	kfree(param_headers);
>  	if (err)
> -		memcpy(nor->params, &params, sizeof(*nor->params));
> +		*nor->params = save_params[0];
>  
>  	return err;
>  }

[Severity: Medium]
Is it safe to mix scope-based cleanup with goto labels in this function?

The patch introduces a scope-based cleanup helper __free(kfree) for
save_params, but leaves multiple existing goto error labels (such as
free_param_headers, free_sfdp, and free_dwords) untouched in
spi_nor_parse_sfdp().

The cleanup subsystem guidelines state that the expectation is that usage of
goto and cleanup helpers is never mixed in the same function. 

Could this lead to confusing ownership semantics, and should all resources in
this routine be converted to scope-based cleanup (or none of them)?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261015090516.924703-1-mwalle@kernel.org?part=1

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/