[PATCH 0/2] mtd: spi-nor: sfdp: bound two optional parameter tables

[PATCH 0/2] mtd: spi-nor: sfdp: bound two optional parameter tables

[HyeongJun An]

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

  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 length is a u8 supplied by the flash, so a device that advertises
either table with a shorter length makes the read run past the
allocation, and a length of zero makes kmalloc() hand back
ZERO_SIZE_PTR, which the parsers then dereference.

spi_nor_parse_4bait() already guards its table this way with
SFDP_4BAIT_DWORD_MAX, so both patches just apply the same check.
spi_nor_parse_sccr_mc() is left alone: it derives its highest index from
the length it allocated with, so it stays in bounds.

The two parsers were added at different times, so they carry different
Fixes tags and are split into 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.

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


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

[PATCH 1/2] mtd: spi-nor: sfdp: check the length of the xSPI Profile 1.0 table

[HyeongJun An]

spi_nor_parse_profile1() sizes its bounce buffer from the length the
flash reports in the SFDP parameter header, then indexes that buffer at
fixed offsets:

    len = profile1_header->length * sizeof(*dwords);
    dwords = kmalloc(len, GFP_KERNEL);
    ...
    dummy = FIELD_GET(PROFILE1_DWORD5_DUMMY_166MHZ, dwords[SFDP_DWORD(5)]);

The parser reads up to DWORD5, but never checks that the table is that
long.  The length is a u8 the flash supplies, so a device advertising
the table with a shorter length makes the read run past the allocation:
with a length of one the buffer is four bytes and dwords[SFDP_DWORD(5)]
reads sixteen bytes beyond it.  A length of zero is worse, as kmalloc()
then returns ZERO_SIZE_PTR rather than an error and the first access
dereferences it.

The bytes read out of bounds are not just discarded either, they end up
as the dummy cycle count programmed for 8D-8D-8D fast reads.

SFDP tables that do not match what the flash actually implements are
common enough that this file already carries fixup hooks for them, and
malformed SFDP has caused memory-safety bugs here before.  See
commit f0f0cfdc3a02 ("mtd: spi-nor: Fix shift-out-of-bounds in
spi_nor_set_erase_type").

Reject a table shorter than the highest DWORD the parser reads, the way
spi_nor_parse_4bait() already does with SFDP_4BAIT_DWORD_MAX.  Failing
an optional parameter table is not fatal: spi_nor_parse_sfdp() warns and
carries on with the data gathered so far.

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..d3d85b5d1b4a 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_MAX			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_MAX)
+		return -EINVAL;
+
 	len = profile1_header->length * sizeof(*dwords);
 	dwords = kmalloc(len, GFP_KERNEL);
 	if (!dwords)
-- 
2.43.0


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

Re: [PATCH 1/2] mtd: spi-nor: sfdp: check the length of the xSPI Profile 1.0 table

[Michael Walle]

[-- Attachment #1.1: Type: text/plain, Size: 3003 bytes --]

On Sun Jul 19, 2026 at 3:08 AM CEST, HyeongJun An wrote:
> spi_nor_parse_profile1() sizes its bounce buffer from the length the
> flash reports in the SFDP parameter header, then indexes that buffer at
> fixed offsets:
>
>     len = profile1_header->length * sizeof(*dwords);
>     dwords = kmalloc(len, GFP_KERNEL);
>     ...
>     dummy = FIELD_GET(PROFILE1_DWORD5_DUMMY_166MHZ, dwords[SFDP_DWORD(5)]);
>
> The parser reads up to DWORD5, but never checks that the table is that
> long.  The length is a u8 the flash supplies, so a device advertising
> the table with a shorter length makes the read run past the allocation:
> with a length of one the buffer is four bytes and dwords[SFDP_DWORD(5)]
> reads sixteen bytes beyond it.  A length of zero is worse, as kmalloc()
> then returns ZERO_SIZE_PTR rather than an error and the first access
> dereferences it.
>
> The bytes read out of bounds are not just discarded either, they end up
> as the dummy cycle count programmed for 8D-8D-8D fast reads.
>
> SFDP tables that do not match what the flash actually implements are
> common enough that this file already carries fixup hooks for them, and
> malformed SFDP has caused memory-safety bugs here before.  See
> commit f0f0cfdc3a02 ("mtd: spi-nor: Fix shift-out-of-bounds in
> spi_nor_set_erase_type").
>
> Reject a table shorter than the highest DWORD the parser reads, the way
> spi_nor_parse_4bait() already does with SFDP_4BAIT_DWORD_MAX.  Failing
> an optional parameter table is not fatal: spi_nor_parse_sfdp() warns and
> carries on with the data gathered so far.

Please be more precise. Best would be to describe the issue in your
own words.

> 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..d3d85b5d1b4a 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_MAX			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_MAX)
> +		return -EINVAL;
> +

I see that it's also use in the 4bait parsing, but I find it very
confusing to read. IMHO it should be SFDP_PROFILE1_DWORD_MIN.

-michael

>  	len = profile1_header->length * sizeof(*dwords);
>  	dwords = kmalloc(len, GFP_KERNEL);
>  	if (!dwords)


[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 297 bytes --]

[-- Attachment #2: Type: text/plain, Size: 144 bytes --]

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

[PATCH 2/2] mtd: spi-nor: sfdp: check the length of the SCCR map

[HyeongJun An]

spi_nor_parse_sccr() sizes its bounce buffer from the length the flash
reports in the SFDP parameter header, then reads DWORD22 out of it:

    len = sccr_header->length * sizeof(*dwords);
    dwords = kmalloc(len, GFP_KERNEL);
    ...
    if (FIELD_GET(SCCR_DWORD22_OCTAL_DTR_EN_VOLATILE,
                  dwords[SFDP_DWORD(22)]))

Nothing checks that the table actually has 22 DWORDs.  The length is a
u8 the flash supplies, so a device advertising the SCCR map with a
shorter length makes the read run past the allocation: with a length of
one the buffer is four bytes and dwords[SFDP_DWORD(22)] reads
eighty-four bytes beyond it.  A length of zero is worse, as kmalloc()
then returns ZERO_SIZE_PTR rather than an error and the read of
dwords[SFDP_DWORD(1)] dereferences it.

Reject a table shorter than the highest DWORD the parser reads, the way
spi_nor_parse_4bait() already does with SFDP_4BAIT_DWORD_MAX.  Failing
an optional parameter table is not fatal: spi_nor_parse_sfdp() warns and
carries on with the data gathered so far.

spi_nor_parse_sccr_mc() does not need the same check.  It derives the
number of dice from the length it allocated with, so its highest index
stays inside the buffer, and a length below two leaves its loop empty.

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 d3d85b5d1b4a..6c9db103a29a 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_MAX				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_MAX)
+		return -EINVAL;
+
 	len = sccr_header->length * sizeof(*dwords);
 	dwords = kmalloc(len, GFP_KERNEL);
 	if (!dwords)
-- 
2.43.0


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