* [PATCH] mtd: rawnand: validate ONFI extended parameter page sections
@ 2026-07-15 8:42 ` Pengpeng Hou
0 siblings, 0 replies; 4+ messages in thread
From: Pengpeng Hou @ 2026-07-15 8:42 UTC (permalink / raw)
To: miquel.raynal, vigneshr
Cc: Pengpeng Hou, richard, linux-mtd, linux-kernel, Huang Shijie,
stable
nand_flash_detect_ext_param_page() allocates a length declared by the ONFI
parameter page. It treats the returned data as a fixed header followed by
variable-length sections. It reads that header and advances over sections
without first proving that the page and each current section fit in the
allocated buffer.
Reject pages shorter than the fixed header. Track remaining bytes while
walking sections. Require the ECC section to cover every field read from
struct onfi_ext_ecc_info.
Fixes: 6dcbe0cdd83f ("mtd: get the ECC info from the Extended Parameter Page")
Cc: stable@vger.kernel.org
Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
---
drivers/mtd/nand/raw/nand_onfi.c | 22 ++++++++++++++++++++--
1 file changed, 20 insertions(+), 2 deletions(-)
diff --git a/drivers/mtd/nand/raw/nand_onfi.c b/drivers/mtd/nand/raw/nand_onfi.c
index cd3ad373883e..40b7db10bd73 100644
--- a/drivers/mtd/nand/raw/nand_onfi.c
+++ b/drivers/mtd/nand/raw/nand_onfi.c
@@ -40,11 +40,16 @@ static int nand_flash_detect_ext_param_page(struct nand_chip *chip,
struct onfi_ext_section *s;
struct onfi_ext_ecc_info *ecc;
uint8_t *cursor;
+ size_t section_len;
+ size_t remaining;
int ret;
int len;
int i;
len = le16_to_cpu(p->ext_param_page_length) * 16;
+ if (len < sizeof(*ep))
+ return -EINVAL;
+
ep = kmalloc(len, GFP_KERNEL);
if (!ep)
return -ENOMEM;
@@ -77,11 +82,24 @@ static int nand_flash_detect_ext_param_page(struct nand_chip *chip,
/* find the ECC section. */
cursor = (uint8_t *)(ep + 1);
+ remaining = len - sizeof(*ep);
for (i = 0; i < ONFI_EXT_SECTION_MAX; i++) {
s = ep->sections + i;
- if (s->type == ONFI_SECTION_TYPE_2)
+ section_len = s->length * 16;
+ if (section_len > remaining) {
+ pr_debug("The section is invalid.\n");
+ goto ext_out;
+ }
+
+ if (s->type == ONFI_SECTION_TYPE_2) {
+ if (section_len < sizeof(*ecc)) {
+ pr_debug("The ECC section is invalid.\n");
+ goto ext_out;
+ }
break;
- cursor += s->length * 16;
+ }
+ cursor += section_len;
+ remaining -= section_len;
}
if (i == ONFI_EXT_SECTION_MAX) {
pr_debug("We can not find the ECC section.\n");
--
2.43.0
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply related [flat|nested] 4+ messages in thread* [PATCH] mtd: rawnand: validate ONFI extended parameter page sections
@ 2026-07-15 8:42 ` Pengpeng Hou
0 siblings, 0 replies; 4+ messages in thread
From: Pengpeng Hou @ 2026-07-15 8:42 UTC (permalink / raw)
To: miquel.raynal, vigneshr
Cc: Pengpeng Hou, richard, linux-mtd, linux-kernel, Huang Shijie,
stable
nand_flash_detect_ext_param_page() allocates a length declared by the ONFI
parameter page. It treats the returned data as a fixed header followed by
variable-length sections. It reads that header and advances over sections
without first proving that the page and each current section fit in the
allocated buffer.
Reject pages shorter than the fixed header. Track remaining bytes while
walking sections. Require the ECC section to cover every field read from
struct onfi_ext_ecc_info.
Fixes: 6dcbe0cdd83f ("mtd: get the ECC info from the Extended Parameter Page")
Cc: stable@vger.kernel.org
Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
---
drivers/mtd/nand/raw/nand_onfi.c | 22 ++++++++++++++++++++--
1 file changed, 20 insertions(+), 2 deletions(-)
diff --git a/drivers/mtd/nand/raw/nand_onfi.c b/drivers/mtd/nand/raw/nand_onfi.c
index cd3ad373883e..40b7db10bd73 100644
--- a/drivers/mtd/nand/raw/nand_onfi.c
+++ b/drivers/mtd/nand/raw/nand_onfi.c
@@ -40,11 +40,16 @@ static int nand_flash_detect_ext_param_page(struct nand_chip *chip,
struct onfi_ext_section *s;
struct onfi_ext_ecc_info *ecc;
uint8_t *cursor;
+ size_t section_len;
+ size_t remaining;
int ret;
int len;
int i;
len = le16_to_cpu(p->ext_param_page_length) * 16;
+ if (len < sizeof(*ep))
+ return -EINVAL;
+
ep = kmalloc(len, GFP_KERNEL);
if (!ep)
return -ENOMEM;
@@ -77,11 +82,24 @@ static int nand_flash_detect_ext_param_page(struct nand_chip *chip,
/* find the ECC section. */
cursor = (uint8_t *)(ep + 1);
+ remaining = len - sizeof(*ep);
for (i = 0; i < ONFI_EXT_SECTION_MAX; i++) {
s = ep->sections + i;
- if (s->type == ONFI_SECTION_TYPE_2)
+ section_len = s->length * 16;
+ if (section_len > remaining) {
+ pr_debug("The section is invalid.\n");
+ goto ext_out;
+ }
+
+ if (s->type == ONFI_SECTION_TYPE_2) {
+ if (section_len < sizeof(*ecc)) {
+ pr_debug("The ECC section is invalid.\n");
+ goto ext_out;
+ }
break;
- cursor += s->length * 16;
+ }
+ cursor += section_len;
+ remaining -= section_len;
}
if (i == ONFI_EXT_SECTION_MAX) {
pr_debug("We can not find the ECC section.\n");
--
2.43.0
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] mtd: rawnand: validate ONFI extended parameter page sections
2026-07-15 8:42 ` Pengpeng Hou
@ 2026-07-17 12:21 ` Miquel Raynal
-1 siblings, 0 replies; 4+ messages in thread
From: Miquel Raynal @ 2026-07-17 12:21 UTC (permalink / raw)
To: Pengpeng Hou
Cc: vigneshr, richard, linux-mtd, linux-kernel, Huang Shijie, stable
Hi Pengpeng,
On 15/07/2026 at 16:42:26 +08, Pengpeng Hou <pengpeng@iscas.ac.cn> wrote:
> nand_flash_detect_ext_param_page() allocates a length declared by the ONFI
> parameter page. It treats the returned data as a fixed header followed by
> variable-length sections. It reads that header and advances over sections
> without first proving that the page and each current section fit in the
> allocated buffer.
>
> Reject pages shorter than the fixed header. Track remaining bytes while
> walking sections. Require the ECC section to cover every field read from
> struct onfi_ext_ecc_info.
>
> Fixes: 6dcbe0cdd83f ("mtd: get the ECC info from the Extended Parameter Page")
> Cc: stable@vger.kernel.org
> Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
> ---
> drivers/mtd/nand/raw/nand_onfi.c | 22 ++++++++++++++++++++--
> 1 file changed, 20 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/mtd/nand/raw/nand_onfi.c b/drivers/mtd/nand/raw/nand_onfi.c
> index cd3ad373883e..40b7db10bd73 100644
> --- a/drivers/mtd/nand/raw/nand_onfi.c
> +++ b/drivers/mtd/nand/raw/nand_onfi.c
> @@ -40,11 +40,16 @@ static int nand_flash_detect_ext_param_page(struct nand_chip *chip,
> struct onfi_ext_section *s;
> struct onfi_ext_ecc_info *ecc;
> uint8_t *cursor;
> + size_t section_len;
> + size_t remaining;
> int ret;
> int len;
> int i;
>
> len = le16_to_cpu(p->ext_param_page_length) * 16;
> + if (len < sizeof(*ep))
> + return -EINVAL;
> +
> ep = kmalloc(len, GFP_KERNEL);
> if (!ep)
> return -ENOMEM;
> @@ -77,11 +82,24 @@ static int nand_flash_detect_ext_param_page(struct nand_chip *chip,
>
> /* find the ECC section. */
> cursor = (uint8_t *)(ep + 1);
> + remaining = len - sizeof(*ep);
> for (i = 0; i < ONFI_EXT_SECTION_MAX; i++) {
> s = ep->sections + i;
> - if (s->type == ONFI_SECTION_TYPE_2)
> + section_len = s->length * 16;
> + if (section_len > remaining) {
> + pr_debug("The section is invalid.\n");
This debug message is not useful, it could mention the section, and at
least contain "ONFI" in it. But since you have access to a nand_chip, it
is also probably better to use dev_dbg here.
> + goto ext_out;
> + }
> +
> + if (s->type == ONFI_SECTION_TYPE_2) {
> + if (section_len < sizeof(*ecc)) {
> + pr_debug("The ECC section is
> invalid.\n");
Same.
> + goto ext_out;
> + }
> break;
> - cursor += s->length * 16;
> + }
\n
> + cursor += section_len;
> + remaining -= section_len;
> }
> if (i == ONFI_EXT_SECTION_MAX) {
> pr_debug("We can not find the ECC section.\n");
Chances are this will never happen, because it is hardware hard to
tamper with, covered by an initial CRC, and if the manufacturers mess up
so badly their tables the chips won't be manufactured anyway.
But, heh, the cost of this is tolerable. Nevertheless, a v2 is expected.
Thanks,
Miquèl
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] mtd: rawnand: validate ONFI extended parameter page sections
@ 2026-07-17 12:21 ` Miquel Raynal
0 siblings, 0 replies; 4+ messages in thread
From: Miquel Raynal @ 2026-07-17 12:21 UTC (permalink / raw)
To: Pengpeng Hou
Cc: vigneshr, richard, linux-mtd, linux-kernel, Huang Shijie, stable
Hi Pengpeng,
On 15/07/2026 at 16:42:26 +08, Pengpeng Hou <pengpeng@iscas.ac.cn> wrote:
> nand_flash_detect_ext_param_page() allocates a length declared by the ONFI
> parameter page. It treats the returned data as a fixed header followed by
> variable-length sections. It reads that header and advances over sections
> without first proving that the page and each current section fit in the
> allocated buffer.
>
> Reject pages shorter than the fixed header. Track remaining bytes while
> walking sections. Require the ECC section to cover every field read from
> struct onfi_ext_ecc_info.
>
> Fixes: 6dcbe0cdd83f ("mtd: get the ECC info from the Extended Parameter Page")
> Cc: stable@vger.kernel.org
> Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
> ---
> drivers/mtd/nand/raw/nand_onfi.c | 22 ++++++++++++++++++++--
> 1 file changed, 20 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/mtd/nand/raw/nand_onfi.c b/drivers/mtd/nand/raw/nand_onfi.c
> index cd3ad373883e..40b7db10bd73 100644
> --- a/drivers/mtd/nand/raw/nand_onfi.c
> +++ b/drivers/mtd/nand/raw/nand_onfi.c
> @@ -40,11 +40,16 @@ static int nand_flash_detect_ext_param_page(struct nand_chip *chip,
> struct onfi_ext_section *s;
> struct onfi_ext_ecc_info *ecc;
> uint8_t *cursor;
> + size_t section_len;
> + size_t remaining;
> int ret;
> int len;
> int i;
>
> len = le16_to_cpu(p->ext_param_page_length) * 16;
> + if (len < sizeof(*ep))
> + return -EINVAL;
> +
> ep = kmalloc(len, GFP_KERNEL);
> if (!ep)
> return -ENOMEM;
> @@ -77,11 +82,24 @@ static int nand_flash_detect_ext_param_page(struct nand_chip *chip,
>
> /* find the ECC section. */
> cursor = (uint8_t *)(ep + 1);
> + remaining = len - sizeof(*ep);
> for (i = 0; i < ONFI_EXT_SECTION_MAX; i++) {
> s = ep->sections + i;
> - if (s->type == ONFI_SECTION_TYPE_2)
> + section_len = s->length * 16;
> + if (section_len > remaining) {
> + pr_debug("The section is invalid.\n");
This debug message is not useful, it could mention the section, and at
least contain "ONFI" in it. But since you have access to a nand_chip, it
is also probably better to use dev_dbg here.
> + goto ext_out;
> + }
> +
> + if (s->type == ONFI_SECTION_TYPE_2) {
> + if (section_len < sizeof(*ecc)) {
> + pr_debug("The ECC section is
> invalid.\n");
Same.
> + goto ext_out;
> + }
> break;
> - cursor += s->length * 16;
> + }
\n
> + cursor += section_len;
> + remaining -= section_len;
> }
> if (i == ONFI_EXT_SECTION_MAX) {
> pr_debug("We can not find the ECC section.\n");
Chances are this will never happen, because it is hardware hard to
tamper with, covered by an initial CRC, and if the manufacturers mess up
so badly their tables the chips won't be manufactured anyway.
But, heh, the cost of this is tolerable. Nevertheless, a v2 is expected.
Thanks,
Miquèl
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-07-17 12:21 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-15 8:42 [PATCH] mtd: rawnand: validate ONFI extended parameter page sections Pengpeng Hou
2026-07-15 8:42 ` Pengpeng Hou
2026-07-17 12:21 ` Miquel Raynal
2026-07-17 12:21 ` Miquel Raynal
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.