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

[PATCH v2] 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>
---
v2:
 - don't use the cleanup helpers to not mix them with gotos as
   suggested by Miquel. I didn't carry your Rb over because there
   is quite a bit more code change.

 drivers/mtd/spi-nor/sfdp.c | 36 ++++++++++++++++++++++++------------
 1 file changed, 24 insertions(+), 12 deletions(-)

diff --git a/drivers/mtd/spi-nor/sfdp.c b/drivers/mtd/spi-nor/sfdp.c
index beb0f5c66212..2c96f7820188 100644
--- a/drivers/mtd/spi-nor/sfdp.c
+++ b/drivers/mtd/spi-nor/sfdp.c
@@ -1537,7 +1537,7 @@ int spi_nor_parse_sfdp(struct spi_nor *nor)
 {
 	const struct sfdp_parameter_header *param_header, *bfpt_header;
 	struct sfdp_parameter_header *param_headers = NULL;
-	struct spi_nor_flash_parameter params, params2;
+	struct spi_nor_flash_parameter *save_params;
 	struct sfdp_header header;
 	struct device *dev = nor->dev;
 	struct sfdp *sfdp;
@@ -1549,17 +1549,22 @@ 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));
+	save_params = kmalloc_objs(*save_params, 2);
+	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);
 	if (err < 0)
-		return err;
+		goto free_save_params;
 
 	/* Check the SFDP header version. */
 	if (le32_to_cpu(header.signature) != SFDP_SIGNATURE ||
-	    header.major != SFDP_JESD216_MAJOR)
-		return -EINVAL;
+	    header.major != SFDP_JESD216_MAJOR) {
+		err = -EINVAL;
+		goto free_save_params;
+	}
 
 	/*
 	 * Verify that the first and only mandatory parameter header is a
@@ -1567,8 +1572,10 @@ int spi_nor_parse_sfdp(struct spi_nor *nor)
 	 */
 	bfpt_header = &header.bfpt_header;
 	if (SFDP_PARAM_HEADER_ID(bfpt_header) != SFDP_BFPT_ID ||
-	    bfpt_header->major != SFDP_JESD216_MAJOR)
-		return -EINVAL;
+	    bfpt_header->major != SFDP_JESD216_MAJOR) {
+		err = -EINVAL;
+		goto free_save_params;
+	}
 
 	sfdp_size = SFDP_PARAM_HEADER_PTP(bfpt_header) +
 		    SFDP_PARAM_HEADER_PARAM_LEN(bfpt_header);
@@ -1588,8 +1595,10 @@ int spi_nor_parse_sfdp(struct spi_nor *nor)
 		psize = header.nph * sizeof(*param_headers);
 
 		param_headers = kmalloc(psize, GFP_KERNEL);
-		if (!param_headers)
-			return -ENOMEM;
+		if (!param_headers) {
+			err = -ENOMEM;
+			goto free_save_params;
+		}
 
 		err = spi_nor_read_sfdp(nor, sizeof(header),
 					psize, param_headers);
@@ -1665,7 +1674,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 +1712,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];
 		}
 	}
 
@@ -1712,6 +1721,7 @@ int spi_nor_parse_sfdp(struct spi_nor *nor)
 		goto clear_sfdp_ptr;
 
 	kfree(param_headers);
+	kfree(save_params);
 
 	return 0;
 
@@ -1724,7 +1734,9 @@ 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];
+free_save_params:
+	kfree(save_params);
 
 	return err;
 }
-- 
2.47.3


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

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

[Miquel Raynal]

On 15/10/2026 at 13:31:52 +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>
> ---
> v2:
>  - don't use the cleanup helpers to not mix them with gotos as
>    suggested by Miquel. I didn't carry your Rb over because there
>    is quite a bit more code change.

Would have been fine.

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

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