mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v2 0/2] mtd: spi-nor: sfdp: bound two optional parameter tables
@ 2026-07-20 14:11 HyeongJun An
  2026-07-20 14:11 ` [PATCH v2 1/2] mtd: spi-nor: sfdp: check the length of the xSPI Profile 1.0 table HyeongJun An
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: HyeongJun An @ 2026-07-20 14:11 UTC (permalink / raw)
  To: pratyush, mwalle, tudor.ambarus, miquel.raynal, richard, vigneshr
  Cc: takahiro.kuwano, linux-mtd, linux-kernel, stable, HyeongJun An

Two of the optional SFDP parameter table parsers size a buffer from the
table length the flash reports, then index it at fixed offsets without
checking the table is long enough.

  spi_nor_parse_profile1()  reads up to DWORD5,  never checks the length
  spi_nor_parse_sccr()      reads up to DWORD22, never checks the length

The spi_nor_parse_4bait() already guards its table this way, so both
patches apply the same check. The two parsers were added at different
times, so they carry different Fixes tags and get one patch each.

Found by code inspection. I have no xSPI or multi-die part to reproduce
this on, so this is not runtime tested.

changes since v1:
 - reworded both commit messages to be more precise
 - renamed the two constants from _DWORD_MAX to _DWORD_MIN
 - Link to v1: https://lore.kernel.org/linux-mtd/20260719010820.1924739-1-sammiee5311@gmail.com/

HyeongJun An (2):
  mtd: spi-nor: sfdp: check the length of the xSPI Profile 1.0 table
  mtd: spi-nor: sfdp: check the length of the SCCR map

 drivers/mtd/spi-nor/sfdp.c | 8 ++++++++
 1 file changed, 8 insertions(+)

-- 
2.43.0


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH v2 1/2] mtd: spi-nor: sfdp: check the length of the xSPI Profile 1.0 table
  2026-07-20 14:11 [PATCH v2 0/2] mtd: spi-nor: sfdp: bound two optional parameter tables HyeongJun An
@ 2026-07-20 14:11 ` HyeongJun An
  2026-07-20 14:11 ` [PATCH v2 2/2] mtd: spi-nor: sfdp: check the length of the SCCR map HyeongJun An
  2026-09-04 13:40 ` [PATCH v2 0/2] mtd: spi-nor: sfdp: bound two optional parameter tables Michael Walle
  2 siblings, 0 replies; 4+ messages in thread
From: HyeongJun An @ 2026-07-20 14:11 UTC (permalink / raw)
  To: pratyush, mwalle, tudor.ambarus, miquel.raynal, richard, vigneshr
  Cc: takahiro.kuwano, linux-mtd, linux-kernel, stable, HyeongJun An

The spi_nor_parse_profile1() sizes its buffer from the table length the
flash reports in the SFDP parameter header. But it then reads DWORD1,
DWORD4 and DWORD5 without ever checking the table is that long.

So if a flash reports a length of one, the buffer is only four bytes
while DWORD4 and DWORD5 sit at byte offsets 12 and 16. With a length of
zero kmalloc() returns ZERO_SIZE_PTR rather than an error, so the NULL
check doesn't catch it and the first read dereferences it. And the value
doesn't just get thrown away. It ends up as the dummy cycle count for
8D-8D-8D fast reads.

To fix this, reject a table that's too short for the highest DWORD the
parser reads, the way spi_nor_parse_4bait() already does. The table is
optional, so this isn't fatal. The spi_nor_parse_sfdp() warns and
carries on.

Fixes: fb27f198971a ("mtd: spi-nor: sfdp: parse xSPI Profile 1.0 table")
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: HyeongJun An <sammiee5311@gmail.com>
---
 drivers/mtd/spi-nor/sfdp.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/mtd/spi-nor/sfdp.c b/drivers/mtd/spi-nor/sfdp.c
index 4600983cb579..ece8bbd4bc47 100644
--- a/drivers/mtd/spi-nor/sfdp.c
+++ b/drivers/mtd/spi-nor/sfdp.c
@@ -1175,6 +1175,7 @@ static int spi_nor_parse_4bait(struct spi_nor *nor,
 #define PROFILE1_DWORD5_DUMMY_166MHZ		GENMASK(31, 27)
 #define PROFILE1_DWORD5_DUMMY_133MHZ		GENMASK(21, 17)
 #define PROFILE1_DWORD5_DUMMY_100MHZ		GENMASK(11, 7)
+#define SFDP_PROFILE1_DWORD_MIN			5
 
 /**
  * spi_nor_parse_profile1() - parse the xSPI Profile 1.0 table
@@ -1192,6 +1193,9 @@ static int spi_nor_parse_profile1(struct spi_nor *nor,
 	int ret;
 	u8 dummy, opcode;
 
+	if (profile1_header->length < SFDP_PROFILE1_DWORD_MIN)
+		return -EINVAL;
+
 	len = profile1_header->length * sizeof(*dwords);
 	dwords = kmalloc(len, GFP_KERNEL);
 	if (!dwords)
-- 
2.43.0


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH v2 2/2] mtd: spi-nor: sfdp: check the length of the SCCR map
  2026-07-20 14:11 [PATCH v2 0/2] mtd: spi-nor: sfdp: bound two optional parameter tables HyeongJun An
  2026-07-20 14:11 ` [PATCH v2 1/2] mtd: spi-nor: sfdp: check the length of the xSPI Profile 1.0 table HyeongJun An
@ 2026-07-20 14:11 ` HyeongJun An
  2026-09-04 13:40 ` [PATCH v2 0/2] mtd: spi-nor: sfdp: bound two optional parameter tables Michael Walle
  2 siblings, 0 replies; 4+ messages in thread
From: HyeongJun An @ 2026-07-20 14:11 UTC (permalink / raw)
  To: pratyush, mwalle, tudor.ambarus, miquel.raynal, richard, vigneshr
  Cc: takahiro.kuwano, linux-mtd, linux-kernel, stable, HyeongJun An

The spi_nor_parse_sccr() sizes its buffer from the table length the
flash reports in the SFDP parameter header. But it then reads DWORD1
and DWORD22 without ever checking the table is that long.

So if a flash reports a length of one, the buffer is only four bytes
while DWORD22 sits at byte offset 84. With a length of zero kmalloc()
returns ZERO_SIZE_PTR rather than an error, so the NULL check doesn't
catch it and the first read dereferences it.

To fix this, reject a table that's too short for the highest DWORD the
parser reads, the way spi_nor_parse_4bait() already does. The table is
optional, so this isn't fatal. The spi_nor_parse_sfdp() warns and
carries on.

The spi_nor_parse_sccr_mc() doesn't need the same check. It works out
the number of dice from the length it allocated with, so its highest
index stays inside the buffer.

Fixes: 7ab8b810757a ("mtd: spi-nor: sfdp: Add support for SCCR map for multi-chip device")
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: HyeongJun An <sammiee5311@gmail.com>
---
 drivers/mtd/spi-nor/sfdp.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/mtd/spi-nor/sfdp.c b/drivers/mtd/spi-nor/sfdp.c
index ece8bbd4bc47..98559536d41b 100644
--- a/drivers/mtd/spi-nor/sfdp.c
+++ b/drivers/mtd/spi-nor/sfdp.c
@@ -1266,6 +1266,7 @@ static int spi_nor_parse_profile1(struct spi_nor *nor,
 }
 
 #define SCCR_DWORD22_OCTAL_DTR_EN_VOLATILE		BIT(31)
+#define SFDP_SCCR_DWORD_MIN				22
 
 /**
  * spi_nor_parse_sccr() - Parse the Status, Control and Configuration Register
@@ -1284,6 +1285,9 @@ static int spi_nor_parse_sccr(struct spi_nor *nor,
 	size_t len;
 	int ret;
 
+	if (sccr_header->length < SFDP_SCCR_DWORD_MIN)
+		return -EINVAL;
+
 	len = sccr_header->length * sizeof(*dwords);
 	dwords = kmalloc(len, GFP_KERNEL);
 	if (!dwords)
-- 
2.43.0


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2 0/2] mtd: spi-nor: sfdp: bound two optional parameter tables
  2026-07-20 14:11 [PATCH v2 0/2] mtd: spi-nor: sfdp: bound two optional parameter tables HyeongJun An
  2026-07-20 14:11 ` [PATCH v2 1/2] mtd: spi-nor: sfdp: check the length of the xSPI Profile 1.0 table HyeongJun An
  2026-07-20 14:11 ` [PATCH v2 2/2] mtd: spi-nor: sfdp: check the length of the SCCR map HyeongJun An
@ 2026-09-04 13:40 ` Michael Walle
  2 siblings, 0 replies; 4+ messages in thread
From: Michael Walle @ 2026-09-04 13:40 UTC (permalink / raw)
  To: pratyush, tudor.ambarus, miquel.raynal, richard, vigneshr, HyeongJun An
  Cc: takahiro.kuwano, linux-mtd, linux-kernel, stable


On Mon, 20 Jul 2026 23:11:02 +0900, HyeongJun An wrote:
> Two of the optional SFDP parameter table parsers size a buffer from the
> table length the flash reports, then index it at fixed offsets without
> checking the table is long enough.
> 
>   spi_nor_parse_profile1()  reads up to DWORD5,  never checks the length
>   spi_nor_parse_sccr()      reads up to DWORD22, never checks the length
> 
> [...]

Applied, thanks!

[1/2] mtd: spi-nor: sfdp: check the length of the xSPI Profile 1.0 table
      commit: b9f34a95a81021fa1f3c9cb8cfd458e66824cad6
[2/2] mtd: spi-nor: sfdp: check the length of the SCCR map
      commit: 1413f38f37194673502e61cd3749e091c031370b

Best regards,
-- 
Michael Walle <mwalle@kernel.org>


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-09-04 13:40 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-07-20 14:11 [PATCH v2 0/2] mtd: spi-nor: sfdp: bound two optional parameter tables HyeongJun An
2026-07-20 14:11 ` [PATCH v2 1/2] mtd: spi-nor: sfdp: check the length of the xSPI Profile 1.0 table HyeongJun An
2026-07-20 14:11 ` [PATCH v2 2/2] mtd: spi-nor: sfdp: check the length of the SCCR map HyeongJun An
2026-09-04 13:40 ` [PATCH v2 0/2] mtd: spi-nor: sfdp: bound two optional parameter tables Michael Walle

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®