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

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

[Michael Walle]

Two large spi_nor_flash_parameter structures on a function stack
are really too much, and this can exceed an otherwise reasonable
frame limit:

drivers/mtd/spi-nor/sfdp.c: In function 'spi_nor_parse_sfdp':
drivers/mtd/spi-nor/sfdp.c:1717:1: error: the frame size of 1600 bytes is larger than 1536 bytes [-Werror=frame-larger-than=]

Allocate both rollback copies on the heap and use structure copies
for type safety.

Suggested-by: Arnd Bergmann <arnd@arndb.de>
Suggested-by: David Laight <david.laight.linux@gmail.com>
Fixes: d20029474a76 ("mtd: spi-nor: push the rollback mechanism into the sfdp module")
Signed-off-by: Michael Walle <mwalle@kernel.org>
---
 drivers/mtd/spi-nor/sfdp.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/mtd/spi-nor/sfdp.c b/drivers/mtd/spi-nor/sfdp.c
index beb0f5c66212..e30cdb5b18ed 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;
 	struct sfdp_parameter_header *param_headers = NULL;
-	struct spi_nor_flash_parameter params, params2;
 	struct sfdp_header header;
 	struct device *dev = nor->dev;
 	struct sfdp *sfdp;
@@ -1549,7 +1549,9 @@ int spi_nor_parse_sfdp(struct spi_nor *nor)
 	 * Get a backup of all the parameter to roll back to in case of an
 	 * error.
 	 */
-	memcpy(&params, nor->params, sizeof(params));
+	if (!save_params)
+		return -ENOMEM;
+	save_params[0] = *nor->params;
 
 	/* Get the SFDP header. */
 	err = spi_nor_read_sfdp_dma_unsafe(nor, 0, sizeof(header), &header);
@@ -1665,7 +1667,7 @@ int spi_nor_parse_sfdp(struct spi_nor *nor)
 
 	/* Parse optional parameter tables. */
 	for (i = 0; i < header.nph; i++) {
-		memcpy(&params2, nor->params, sizeof(params2));
+		save_params[1] = *nor->params;
 		param_header = &param_headers[i];
 
 		switch (SFDP_PARAM_HEADER_ID(param_header)) {
@@ -1703,7 +1705,7 @@ int spi_nor_parse_sfdp(struct spi_nor *nor)
 			 * spi_nor_flash_parameter data.
 			 */
 			err = 0;
-			memcpy(nor->params, &params2, sizeof(*nor->params));
+			*nor->params = save_params[1];
 		}
 	}
 
@@ -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;
 }
-- 
2.47.3


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

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

[sashiko-bot]

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/

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

[Miquel Raynal]

On 15/10/2026 at 11:01:28 +02, Michael Walle <mwalle@kernel.org> wrote:

> Two large spi_nor_flash_parameter structures on a function stack
> are really too much, and this can exceed an otherwise reasonable
> frame limit:
>
> drivers/mtd/spi-nor/sfdp.c: In function 'spi_nor_parse_sfdp':
> drivers/mtd/spi-nor/sfdp.c:1717:1: error: the frame size of 1600 bytes is larger than 1536 bytes [-Werror=frame-larger-than=]
>
> Allocate both rollback copies on the heap and use structure copies
> for type safety.
>
> Suggested-by: Arnd Bergmann <arnd@arndb.de>
> Suggested-by: David Laight <david.laight.linux@gmail.com>
> Fixes: d20029474a76 ("mtd: spi-nor: push the rollback mechanism into the sfdp module")
> Signed-off-by: Michael Walle <mwalle@kernel.org>

Sashiko is correct about the need for not mixing __free and goto's. With
this fixed,

Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>

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