All of lore.kernel.org
 help / color / mirror / Atom feed
From: Johan Jonker <jbx6244@gmail.com>
To: miquel.raynal@bootlin.com
Cc: richard@nod.at, vigneshr@ti.com, heiko@sntech.de,
	linux-mtd@lists.infradead.org, linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-rockchip@lists.infradead.org, yifeng.zhao@rock-chips.com
Subject: [PATCH v3 1/3] mtd: rawnand: rockchip-nand-controller: fix oobfree offset and description
Date: Thu, 15 Jun 2023 19:34:01 +0200	[thread overview]
Message-ID: <dfb76ad5-b62a-3f1d-494e-cd17d57945ae@gmail.com> (raw)
In-Reply-To: <0047fc52-bc45-a768-8bdd-c0f12cddc17e@gmail.com>

The MTD framework reserves 1 or 2 bytes for the bad block marker
depending on the bus size. The rockchip-nand-controller driver
currently only supports a 8 bit bus, but reserves standard 2 bytes
for the BBM in the chip->oob_poi buffer. The first free OOB byte is
therefore OOB2 at offset 2. Page Address (PA) bytes are located at the
last 4 positions before ECC. The current advertised free OOB area has
an offset that starts at OOB6 and a length that overlaps with the space
reserved for the PA bytes. Writing unrelated data to a reserved space
with a specific task can corrupt our boot block page read order.
Fix by changing the free OOB offset to 2.

This change breaks existing jffs2 users.

Signed-off-by: Johan Jonker <jbx6244@gmail.com>
---

Changed V3:
  Change prefixes
  Reword
  State break existing users.

---

Example:

Wrong free OOB offset starts at OOB6:
oob_region->offset = NFC_SYS_DATA_SIZE + 2;
                   = 4 + 2
                   = 6

oob_region->length = rknand->metadata_size - NFC_SYS_DATA_SIZE - 2;
                   = 32 - 4 - 2
                   = 26

Together with this length above it overlaps a reserved space for the
boot blocks Page Address(PA)

chip->oob_poi buffer layout for 8 steps:

BBM0   BBM1  OOB2  OOB3  | OOB4  OOB5  OOB6  OOB7

OOB8   OOB9  OOB10 OOB11 | OOB12 OOB13 OOB15 OOB15
OOB16  OOB17 OOB18 OOB19 | OOB20 OOB21 OOB22 OOB23

OOB24  OOB25 OOB26 OOB27 | PA0   PA1   PA2   PA3

ECC0   ECC1  ECC2  ECC3  | ...   ...   ...   ...

Fix by new offset at OOB2:
oob_region->offset = 2;

The full range of free OOB with 8 steps runs from OOB2
till/including OOB27.
---
 drivers/mtd/nand/raw/rockchip-nand-controller.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/drivers/mtd/nand/raw/rockchip-nand-controller.c b/drivers/mtd/nand/raw/rockchip-nand-controller.c
index 2312e2736..37fc07ba5 100644
--- a/drivers/mtd/nand/raw/rockchip-nand-controller.c
+++ b/drivers/mtd/nand/raw/rockchip-nand-controller.c
@@ -562,9 +562,10 @@ static int rk_nfc_write_page_raw(struct nand_chip *chip, const u8 *buf,
 		 *    BBM  OOB1 OOB2 OOB3 |......|  PA0  PA1  PA2  PA3
 		 *
 		 * The rk_nfc_ooblayout_free() function already has reserved
-		 * these 4 bytes with:
+		 * these 4 bytes together with 2 bytes for BBM
+		 * by reducing it's length:
 		 *
-		 * oob_region->offset = NFC_SYS_DATA_SIZE + 2;
+		 * oob_region->length = rknand->metadata_size - NFC_SYS_DATA_SIZE - 2;
 		 */
 		if (!i)
 			memcpy(rk_nfc_oob_ptr(chip, i),
@@ -933,12 +934,8 @@ static int rk_nfc_ooblayout_free(struct mtd_info *mtd, int section,
 	if (section)
 		return -ERANGE;

-	/*
-	 * The beginning of the OOB area stores the reserved data for the NFC,
-	 * the size of the reserved data is NFC_SYS_DATA_SIZE bytes.
-	 */
 	oob_region->length = rknand->metadata_size - NFC_SYS_DATA_SIZE - 2;
-	oob_region->offset = NFC_SYS_DATA_SIZE + 2;
+	oob_region->offset = 2;

 	return 0;
 }
--
2.30.2


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

WARNING: multiple messages have this Message-ID (diff)
From: Johan Jonker <jbx6244@gmail.com>
To: miquel.raynal@bootlin.com
Cc: richard@nod.at, vigneshr@ti.com, heiko@sntech.de,
	linux-mtd@lists.infradead.org, linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-rockchip@lists.infradead.org, yifeng.zhao@rock-chips.com
Subject: [PATCH v3 1/3] mtd: rawnand: rockchip-nand-controller: fix oobfree offset and description
Date: Thu, 15 Jun 2023 19:34:01 +0200	[thread overview]
Message-ID: <dfb76ad5-b62a-3f1d-494e-cd17d57945ae@gmail.com> (raw)
In-Reply-To: <0047fc52-bc45-a768-8bdd-c0f12cddc17e@gmail.com>

The MTD framework reserves 1 or 2 bytes for the bad block marker
depending on the bus size. The rockchip-nand-controller driver
currently only supports a 8 bit bus, but reserves standard 2 bytes
for the BBM in the chip->oob_poi buffer. The first free OOB byte is
therefore OOB2 at offset 2. Page Address (PA) bytes are located at the
last 4 positions before ECC. The current advertised free OOB area has
an offset that starts at OOB6 and a length that overlaps with the space
reserved for the PA bytes. Writing unrelated data to a reserved space
with a specific task can corrupt our boot block page read order.
Fix by changing the free OOB offset to 2.

This change breaks existing jffs2 users.

Signed-off-by: Johan Jonker <jbx6244@gmail.com>
---

Changed V3:
  Change prefixes
  Reword
  State break existing users.

---

Example:

Wrong free OOB offset starts at OOB6:
oob_region->offset = NFC_SYS_DATA_SIZE + 2;
                   = 4 + 2
                   = 6

oob_region->length = rknand->metadata_size - NFC_SYS_DATA_SIZE - 2;
                   = 32 - 4 - 2
                   = 26

Together with this length above it overlaps a reserved space for the
boot blocks Page Address(PA)

chip->oob_poi buffer layout for 8 steps:

BBM0   BBM1  OOB2  OOB3  | OOB4  OOB5  OOB6  OOB7

OOB8   OOB9  OOB10 OOB11 | OOB12 OOB13 OOB15 OOB15
OOB16  OOB17 OOB18 OOB19 | OOB20 OOB21 OOB22 OOB23

OOB24  OOB25 OOB26 OOB27 | PA0   PA1   PA2   PA3

ECC0   ECC1  ECC2  ECC3  | ...   ...   ...   ...

Fix by new offset at OOB2:
oob_region->offset = 2;

The full range of free OOB with 8 steps runs from OOB2
till/including OOB27.
---
 drivers/mtd/nand/raw/rockchip-nand-controller.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/drivers/mtd/nand/raw/rockchip-nand-controller.c b/drivers/mtd/nand/raw/rockchip-nand-controller.c
index 2312e2736..37fc07ba5 100644
--- a/drivers/mtd/nand/raw/rockchip-nand-controller.c
+++ b/drivers/mtd/nand/raw/rockchip-nand-controller.c
@@ -562,9 +562,10 @@ static int rk_nfc_write_page_raw(struct nand_chip *chip, const u8 *buf,
 		 *    BBM  OOB1 OOB2 OOB3 |......|  PA0  PA1  PA2  PA3
 		 *
 		 * The rk_nfc_ooblayout_free() function already has reserved
-		 * these 4 bytes with:
+		 * these 4 bytes together with 2 bytes for BBM
+		 * by reducing it's length:
 		 *
-		 * oob_region->offset = NFC_SYS_DATA_SIZE + 2;
+		 * oob_region->length = rknand->metadata_size - NFC_SYS_DATA_SIZE - 2;
 		 */
 		if (!i)
 			memcpy(rk_nfc_oob_ptr(chip, i),
@@ -933,12 +934,8 @@ static int rk_nfc_ooblayout_free(struct mtd_info *mtd, int section,
 	if (section)
 		return -ERANGE;

-	/*
-	 * The beginning of the OOB area stores the reserved data for the NFC,
-	 * the size of the reserved data is NFC_SYS_DATA_SIZE bytes.
-	 */
 	oob_region->length = rknand->metadata_size - NFC_SYS_DATA_SIZE - 2;
-	oob_region->offset = NFC_SYS_DATA_SIZE + 2;
+	oob_region->offset = 2;

 	return 0;
 }
--
2.30.2


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

WARNING: multiple messages have this Message-ID (diff)
From: Johan Jonker <jbx6244@gmail.com>
To: miquel.raynal@bootlin.com
Cc: richard@nod.at, vigneshr@ti.com, heiko@sntech.de,
	linux-mtd@lists.infradead.org, linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-rockchip@lists.infradead.org, yifeng.zhao@rock-chips.com
Subject: [PATCH v3 1/3] mtd: rawnand: rockchip-nand-controller: fix oobfree offset and description
Date: Thu, 15 Jun 2023 19:34:01 +0200	[thread overview]
Message-ID: <dfb76ad5-b62a-3f1d-494e-cd17d57945ae@gmail.com> (raw)
In-Reply-To: <0047fc52-bc45-a768-8bdd-c0f12cddc17e@gmail.com>

The MTD framework reserves 1 or 2 bytes for the bad block marker
depending on the bus size. The rockchip-nand-controller driver
currently only supports a 8 bit bus, but reserves standard 2 bytes
for the BBM in the chip->oob_poi buffer. The first free OOB byte is
therefore OOB2 at offset 2. Page Address (PA) bytes are located at the
last 4 positions before ECC. The current advertised free OOB area has
an offset that starts at OOB6 and a length that overlaps with the space
reserved for the PA bytes. Writing unrelated data to a reserved space
with a specific task can corrupt our boot block page read order.
Fix by changing the free OOB offset to 2.

This change breaks existing jffs2 users.

Signed-off-by: Johan Jonker <jbx6244@gmail.com>
---

Changed V3:
  Change prefixes
  Reword
  State break existing users.

---

Example:

Wrong free OOB offset starts at OOB6:
oob_region->offset = NFC_SYS_DATA_SIZE + 2;
                   = 4 + 2
                   = 6

oob_region->length = rknand->metadata_size - NFC_SYS_DATA_SIZE - 2;
                   = 32 - 4 - 2
                   = 26

Together with this length above it overlaps a reserved space for the
boot blocks Page Address(PA)

chip->oob_poi buffer layout for 8 steps:

BBM0   BBM1  OOB2  OOB3  | OOB4  OOB5  OOB6  OOB7

OOB8   OOB9  OOB10 OOB11 | OOB12 OOB13 OOB15 OOB15
OOB16  OOB17 OOB18 OOB19 | OOB20 OOB21 OOB22 OOB23

OOB24  OOB25 OOB26 OOB27 | PA0   PA1   PA2   PA3

ECC0   ECC1  ECC2  ECC3  | ...   ...   ...   ...

Fix by new offset at OOB2:
oob_region->offset = 2;

The full range of free OOB with 8 steps runs from OOB2
till/including OOB27.
---
 drivers/mtd/nand/raw/rockchip-nand-controller.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/drivers/mtd/nand/raw/rockchip-nand-controller.c b/drivers/mtd/nand/raw/rockchip-nand-controller.c
index 2312e2736..37fc07ba5 100644
--- a/drivers/mtd/nand/raw/rockchip-nand-controller.c
+++ b/drivers/mtd/nand/raw/rockchip-nand-controller.c
@@ -562,9 +562,10 @@ static int rk_nfc_write_page_raw(struct nand_chip *chip, const u8 *buf,
 		 *    BBM  OOB1 OOB2 OOB3 |......|  PA0  PA1  PA2  PA3
 		 *
 		 * The rk_nfc_ooblayout_free() function already has reserved
-		 * these 4 bytes with:
+		 * these 4 bytes together with 2 bytes for BBM
+		 * by reducing it's length:
 		 *
-		 * oob_region->offset = NFC_SYS_DATA_SIZE + 2;
+		 * oob_region->length = rknand->metadata_size - NFC_SYS_DATA_SIZE - 2;
 		 */
 		if (!i)
 			memcpy(rk_nfc_oob_ptr(chip, i),
@@ -933,12 +934,8 @@ static int rk_nfc_ooblayout_free(struct mtd_info *mtd, int section,
 	if (section)
 		return -ERANGE;

-	/*
-	 * The beginning of the OOB area stores the reserved data for the NFC,
-	 * the size of the reserved data is NFC_SYS_DATA_SIZE bytes.
-	 */
 	oob_region->length = rknand->metadata_size - NFC_SYS_DATA_SIZE - 2;
-	oob_region->offset = NFC_SYS_DATA_SIZE + 2;
+	oob_region->offset = 2;

 	return 0;
 }
--
2.30.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

WARNING: multiple messages have this Message-ID (diff)
From: Johan Jonker <jbx6244@gmail.com>
To: miquel.raynal@bootlin.com
Cc: richard@nod.at, vigneshr@ti.com, heiko@sntech.de,
	linux-mtd@lists.infradead.org, linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-rockchip@lists.infradead.org, yifeng.zhao@rock-chips.com
Subject: [PATCH v3 1/3] mtd: rawnand: rockchip-nand-controller: fix oobfree offset and description
Date: Thu, 15 Jun 2023 19:34:01 +0200	[thread overview]
Message-ID: <dfb76ad5-b62a-3f1d-494e-cd17d57945ae@gmail.com> (raw)
In-Reply-To: <0047fc52-bc45-a768-8bdd-c0f12cddc17e@gmail.com>

The MTD framework reserves 1 or 2 bytes for the bad block marker
depending on the bus size. The rockchip-nand-controller driver
currently only supports a 8 bit bus, but reserves standard 2 bytes
for the BBM in the chip->oob_poi buffer. The first free OOB byte is
therefore OOB2 at offset 2. Page Address (PA) bytes are located at the
last 4 positions before ECC. The current advertised free OOB area has
an offset that starts at OOB6 and a length that overlaps with the space
reserved for the PA bytes. Writing unrelated data to a reserved space
with a specific task can corrupt our boot block page read order.
Fix by changing the free OOB offset to 2.

This change breaks existing jffs2 users.

Signed-off-by: Johan Jonker <jbx6244@gmail.com>
---

Changed V3:
  Change prefixes
  Reword
  State break existing users.

---

Example:

Wrong free OOB offset starts at OOB6:
oob_region->offset = NFC_SYS_DATA_SIZE + 2;
                   = 4 + 2
                   = 6

oob_region->length = rknand->metadata_size - NFC_SYS_DATA_SIZE - 2;
                   = 32 - 4 - 2
                   = 26

Together with this length above it overlaps a reserved space for the
boot blocks Page Address(PA)

chip->oob_poi buffer layout for 8 steps:

BBM0   BBM1  OOB2  OOB3  | OOB4  OOB5  OOB6  OOB7

OOB8   OOB9  OOB10 OOB11 | OOB12 OOB13 OOB15 OOB15
OOB16  OOB17 OOB18 OOB19 | OOB20 OOB21 OOB22 OOB23

OOB24  OOB25 OOB26 OOB27 | PA0   PA1   PA2   PA3

ECC0   ECC1  ECC2  ECC3  | ...   ...   ...   ...

Fix by new offset at OOB2:
oob_region->offset = 2;

The full range of free OOB with 8 steps runs from OOB2
till/including OOB27.
---
 drivers/mtd/nand/raw/rockchip-nand-controller.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/drivers/mtd/nand/raw/rockchip-nand-controller.c b/drivers/mtd/nand/raw/rockchip-nand-controller.c
index 2312e2736..37fc07ba5 100644
--- a/drivers/mtd/nand/raw/rockchip-nand-controller.c
+++ b/drivers/mtd/nand/raw/rockchip-nand-controller.c
@@ -562,9 +562,10 @@ static int rk_nfc_write_page_raw(struct nand_chip *chip, const u8 *buf,
 		 *    BBM  OOB1 OOB2 OOB3 |......|  PA0  PA1  PA2  PA3
 		 *
 		 * The rk_nfc_ooblayout_free() function already has reserved
-		 * these 4 bytes with:
+		 * these 4 bytes together with 2 bytes for BBM
+		 * by reducing it's length:
 		 *
-		 * oob_region->offset = NFC_SYS_DATA_SIZE + 2;
+		 * oob_region->length = rknand->metadata_size - NFC_SYS_DATA_SIZE - 2;
 		 */
 		if (!i)
 			memcpy(rk_nfc_oob_ptr(chip, i),
@@ -933,12 +934,8 @@ static int rk_nfc_ooblayout_free(struct mtd_info *mtd, int section,
 	if (section)
 		return -ERANGE;

-	/*
-	 * The beginning of the OOB area stores the reserved data for the NFC,
-	 * the size of the reserved data is NFC_SYS_DATA_SIZE bytes.
-	 */
 	oob_region->length = rknand->metadata_size - NFC_SYS_DATA_SIZE - 2;
-	oob_region->offset = NFC_SYS_DATA_SIZE + 2;
+	oob_region->offset = 2;

 	return 0;
 }
--
2.30.2


  reply	other threads:[~2023-06-15 17:34 UTC|newest]

Thread overview: 44+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-06-15 17:32 [PATCH v3 0/3] Fixes for Rockchip NAND controller driver Johan Jonker
2023-06-15 17:32 ` Johan Jonker
2023-06-15 17:32 ` Johan Jonker
2023-06-15 17:32 ` Johan Jonker
2023-06-15 17:34 ` Johan Jonker [this message]
2023-06-15 17:34   ` [PATCH v3 1/3] mtd: rawnand: rockchip-nand-controller: fix oobfree offset and description Johan Jonker
2023-06-15 17:34   ` Johan Jonker
2023-06-15 17:34   ` Johan Jonker
2023-07-04 15:03   ` Miquel Raynal
2023-07-04 15:03     ` Miquel Raynal
2023-07-04 15:03     ` Miquel Raynal
2023-07-04 15:03     ` Miquel Raynal
2023-06-15 17:34 ` [PATCH v3 2/3] mtd: rawnand: rockchip-nand-controller: copy hwecc PA data to oob_poi buffer Johan Jonker
2023-06-15 17:34   ` Johan Jonker
2023-06-15 17:34   ` Johan Jonker
2023-06-15 17:34   ` Johan Jonker
2023-07-04 15:08   ` Miquel Raynal
2023-07-04 15:08     ` Miquel Raynal
2023-07-04 15:08     ` Miquel Raynal
2023-07-04 15:08     ` Miquel Raynal
2023-07-05 18:20     ` Johan Jonker
2023-07-05 18:20       ` Johan Jonker
2023-07-05 18:20       ` Johan Jonker
2023-07-05 18:20       ` Johan Jonker
2023-06-15 17:34 ` [PATCH v3 3/3] mtd: rawnand: rockchip-nand-controller: add skipbbt option Johan Jonker
2023-06-15 17:34   ` Johan Jonker
2023-06-15 17:34   ` Johan Jonker
2023-06-15 17:34   ` Johan Jonker
2023-07-04 15:13   ` Miquel Raynal
2023-07-04 15:13     ` Miquel Raynal
2023-07-04 15:13     ` Miquel Raynal
2023-07-04 15:13     ` Miquel Raynal
2023-07-07 15:27     ` Johan Jonker
2023-07-07 15:27       ` Johan Jonker
2023-07-07 15:27       ` Johan Jonker
2023-07-07 15:27       ` Johan Jonker
2023-07-12 13:57       ` Miquel Raynal
2023-07-12 13:57         ` Miquel Raynal
2023-07-12 13:57         ` Miquel Raynal
2023-07-12 13:57         ` Miquel Raynal
2023-07-13 15:17         ` Richard Weinberger
2023-07-13 15:17           ` Richard Weinberger
2023-07-13 15:17           ` Richard Weinberger
2023-07-13 15:17           ` Richard Weinberger

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=dfb76ad5-b62a-3f1d-494e-cd17d57945ae@gmail.com \
    --to=jbx6244@gmail.com \
    --cc=heiko@sntech.de \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mtd@lists.infradead.org \
    --cc=linux-rockchip@lists.infradead.org \
    --cc=miquel.raynal@bootlin.com \
    --cc=richard@nod.at \
    --cc=vigneshr@ti.com \
    --cc=yifeng.zhao@rock-chips.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.