diff mbox series

[v2,3/7] mtd: rawnand: brcmnand: Fix potential out-of-bounds access in oob write

Message ID 20240916-brcmnand-fixes-v2-3-08632f64c8ec@linaro.org
State New
Headers show
Series mtd: nand: brcmnand: Backported fixes from Linux | expand

Commit Message

Linus Walleij Sept. 16, 2024, 9:58 a.m. UTC 
From: William Zhang <william.zhang@broadcom.com>

Backport of upstream Linux
commit 5d53244186c9ac58cb88d76a0958ca55b83a15cd
"mtd: rawnand: brcmnand: Fix potential out-of-bounds access in oob write"

When the oob buffer length is not in multiple of words, the oob write
function does out-of-bounds read on the oob source buffer at the last
iteration. Fix that by always checking length limit on the oob buffer
read and fill with 0xff when reaching the end of the buffer to the oob
registers.

Signed-off-by: William Zhang <william.zhang@broadcom.com>
Reviewed-by: Florian Fainelli <florian.fainelli@broadcom.com>
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
Link: https://lore.kernel.org/linux-mtd/20230706182909.79151-5-william.zhang@broadcom.com
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/mtd/nand/raw/brcmnand/brcmnand.c | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

Comments

William Zhang Sept. 17, 2024, 12:22 a.m. UTC | #1 
> -----Original Message-----
> From: Linus Walleij <linus.walleij@linaro.org>
> Sent: Monday, September 16, 2024 2:59 AM
> To: u-boot@lists.denx.de; Dario Binacchi
> <dario.binacchi@amarulasolutions.com>; Michael Trimarchi
> <michael@amarulasolutions.com>; Anand Gore
> <anand.gore@broadcom.com>; William Zhang
> <william.zhang@broadcom.com>; Kursad Oney
> <kursad.oney@broadcom.com>; Philippe Reynes
> <philippe.reynes@softathome.com>
> Cc: Linus Walleij <linus.walleij@linaro.org>; Florian Fainelli
> <florian.fainelli@broadcom.com>; Miquel Raynal
> <miquel.raynal@bootlin.com>
> Subject: [PATCH v2 3/7] mtd: rawnand: brcmnand: Fix potential out-of-
> bounds access in oob write
>
> From: William Zhang <william.zhang@broadcom.com>
>
> Backport of upstream Linux
> commit 5d53244186c9ac58cb88d76a0958ca55b83a15cd
> "mtd: rawnand: brcmnand: Fix potential out-of-bounds access in oob write"
>
> When the oob buffer length is not in multiple of words, the oob write
> function does out-of-bounds read on the oob source buffer at the last
> iteration. Fix that by always checking length limit on the oob buffer
> read and fill with 0xff when reaching the end of the buffer to the oob
> registers.
>
> Signed-off-by: William Zhang <william.zhang@broadcom.com>
> Reviewed-by: Florian Fainelli <florian.fainelli@broadcom.com>
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> Link: https://lore.kernel.org/linux-mtd/20230706182909.79151-5-
> william.zhang@broadcom.com
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
>  drivers/mtd/nand/raw/brcmnand/brcmnand.c | 18 ++++++++++++++++--
>  1 file changed, 16 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/mtd/nand/raw/brcmnand/brcmnand.c
> b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
> index 46a4107a83a9..60d34bd21f53 100644
> --- a/drivers/mtd/nand/raw/brcmnand/brcmnand.c
> +++ b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
> @@ -1334,19 +1334,33 @@ static int write_oob_to_regs(struct
> brcmnand_controller *ctrl, int i,
>  			     const u8 *oob, int sas, int sector_1k)
>  {
>  	int tbytes = sas << sector_1k;
> -	int j;
> +	int j, k = 0;
> +	u32 last = 0xffffffff;
> +	u8 *plast = (u8 *)&last;
>
>  	/* Adjust OOB values for 1K sector size */
>  	if (sector_1k && (i & 0x01))
>  		tbytes = max(0, tbytes - (int)ctrl->max_oob);
>  	tbytes = min_t(int, tbytes, ctrl->max_oob);
>
> -	for (j = 0; j < tbytes; j += 4)
> +	/*
> +	 * tbytes may not be multiple of words. Make sure we don't read
> out of
> +	 * the boundary and stop at last word.
> +	 */
> +	for (j = 0; (j + 3) < tbytes; j += 4)
>  		oob_reg_write(ctrl, j,
>  				(oob[j + 0] << 24) |
>  				(oob[j + 1] << 16) |
>  				(oob[j + 2] <<  8) |
>  				(oob[j + 3] <<  0));
> +
> +	/* handle the remaing bytes */
> +	while (j < tbytes)
> +		plast[k++] = oob[j++];
> +
> +	if (tbytes & 0x3)
> +		oob_reg_write(ctrl, (tbytes & ~0x3), (__force
> u32)cpu_to_be32(last));
> +
>  	return tbytes;
>  }
>
>
> --
> 2.46.0

Reviewed-by: William Zhang <william.zhang@broadcom.com>
diff mbox series

Patch

diff --git a/drivers/mtd/nand/raw/brcmnand/brcmnand.c b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
index 46a4107a83a9..60d34bd21f53 100644
--- a/drivers/mtd/nand/raw/brcmnand/brcmnand.c
+++ b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
@@ -1334,19 +1334,33 @@  static int write_oob_to_regs(struct brcmnand_controller *ctrl, int i,
 			     const u8 *oob, int sas, int sector_1k)
 {
 	int tbytes = sas << sector_1k;
-	int j;
+	int j, k = 0;
+	u32 last = 0xffffffff;
+	u8 *plast = (u8 *)&last;
 
 	/* Adjust OOB values for 1K sector size */
 	if (sector_1k && (i & 0x01))
 		tbytes = max(0, tbytes - (int)ctrl->max_oob);
 	tbytes = min_t(int, tbytes, ctrl->max_oob);
 
-	for (j = 0; j < tbytes; j += 4)
+	/*
+	 * tbytes may not be multiple of words. Make sure we don't read out of
+	 * the boundary and stop at last word.
+	 */
+	for (j = 0; (j + 3) < tbytes; j += 4)
 		oob_reg_write(ctrl, j,
 				(oob[j + 0] << 24) |
 				(oob[j + 1] << 16) |
 				(oob[j + 2] <<  8) |
 				(oob[j + 3] <<  0));
+
+	/* handle the remaing bytes */
+	while (j < tbytes)
+		plast[k++] = oob[j++];
+
+	if (tbytes & 0x3)
+		oob_reg_write(ctrl, (tbytes & ~0x3), (__force u32)cpu_to_be32(last));
+
 	return tbytes;
 }