[PATCH 1/2] mtd: nand: fix error handling in nand_prog_page_op() #1

[PATCH 1/2] mtd: nand: fix error handling in nand_prog_page_op() #1

[Sascha Hauer]

On success chip->legacy.waitfunc() returns the NAND status byte, but on
failure it returns a negative error code. This was never tested for and
instead the return value was interpreted as NAND status without error
checking. Add the missing error check.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/mtd/nand/raw/nand_base.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
index c33fa1b1847f..d2b2a1f70fde 100644
--- a/drivers/mtd/nand/raw/nand_base.c
+++ b/drivers/mtd/nand/raw/nand_base.c
@@ -1466,6 +1466,8 @@ int nand_prog_page_op(struct nand_chip *chip, unsigned int page,
 		chip->legacy.write_buf(chip, buf, len);
 		chip->legacy.cmdfunc(chip, NAND_CMD_PAGEPROG, -1, -1);
 		status = chip->legacy.waitfunc(chip);
+		if (status < 0)
+			return status;
 	}
 
 	if (status & NAND_STATUS_FAIL)
-- 
2.20.1


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

[PATCH 2/2] mtd: nand: fix error handling in nand_prog_page_op() #2

[Sascha Hauer]

On success nand_exec_prog_page_op() returns the NAND status byte, but on
failure it returns a negative error code. nand_prog_page_op() interprets
the return value as NAND status byte without error checking.  This means
a failure in nand_exec_prog_page_op() can go through unnoticed.

The straight forward fix would be to add the missing error checking. To
clean the code a bit we can move the nand status check to
nand_prog_page_op().  This way we can get rid of the overloaded return
value from nand_exec_prog_page_op() and return a plain error code which
is less error prone.

nand_exec_prog_page_op() is only called from one other place and in this
call the 'prog' parameter is false in which case the nand status check
is skipped, so it's correct to not add the NAND status check there.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/mtd/nand/raw/nand_base.c | 23 ++++++++++-------------
 1 file changed, 10 insertions(+), 13 deletions(-)

diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
index d2b2a1f70fde..73d76f4a262f 100644
--- a/drivers/mtd/nand/raw/nand_base.c
+++ b/drivers/mtd/nand/raw/nand_base.c
@@ -1294,8 +1294,6 @@ static int nand_exec_prog_page_op(struct nand_chip *chip, unsigned int page,
 	};
 	struct nand_operation op = NAND_OPERATION(chip->cur_cs, instrs);
 	int naddrs = nand_fill_column_cycles(chip, addrs, offset_in_page);
-	int ret;
-	u8 status;
 
 	if (naddrs < 0)
 		return naddrs;
@@ -1335,15 +1333,7 @@ static int nand_exec_prog_page_op(struct nand_chip *chip, unsigned int page,
 		op.ninstrs--;
 	}
 
-	ret = nand_exec_op(chip, &op);
-	if (!prog || ret)
-		return ret;
-
-	ret = nand_status_op(chip, &status);
-	if (ret)
-		return ret;
-
-	return status;
+	return nand_exec_op(chip, &op);
 }
 
 /**
@@ -1449,7 +1439,8 @@ int nand_prog_page_op(struct nand_chip *chip, unsigned int page,
 		      unsigned int len)
 {
 	struct mtd_info *mtd = nand_to_mtd(chip);
-	int status;
+	u8 status;
+	int ret;
 
 	if (!len || !buf)
 		return -EINVAL;
@@ -1458,8 +1449,14 @@ int nand_prog_page_op(struct nand_chip *chip, unsigned int page,
 		return -EINVAL;
 
 	if (nand_has_exec_op(chip)) {
-		status = nand_exec_prog_page_op(chip, page, offset_in_page, buf,
+		ret = nand_exec_prog_page_op(chip, page, offset_in_page, buf,
 						len, true);
+		if (ret)
+			return ret;
+
+		ret = nand_status_op(chip, &status);
+		if (ret)
+			return ret;
 	} else {
 		chip->legacy.cmdfunc(chip, NAND_CMD_SEQIN, offset_in_page,
 				     page);
-- 
2.20.1


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

Re: [PATCH 2/2] mtd: nand: fix error handling in nand_prog_page_op() #2

[Miquel Raynal]

On Fri, 2021-02-05 at 14:27:25 UTC, Sascha Hauer wrote:
> On success nand_exec_prog_page_op() returns the NAND status byte, but on
> failure it returns a negative error code. nand_prog_page_op() interprets
> the return value as NAND status byte without error checking.  This means
> a failure in nand_exec_prog_page_op() can go through unnoticed.
> 
> The straight forward fix would be to add the missing error checking. To
> clean the code a bit we can move the nand status check to
> nand_prog_page_op().  This way we can get rid of the overloaded return
> value from nand_exec_prog_page_op() and return a plain error code which
> is less error prone.
> 
> nand_exec_prog_page_op() is only called from one other place and in this
> call the 'prog' parameter is false in which case the nand status check
> is skipped, so it's correct to not add the NAND status check there.
> 
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>

Applied to https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git nand/next, thanks.

Miquel

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

Re: [PATCH 1/2] mtd: nand: fix error handling in nand_prog_page_op() #1

[Miquel Raynal]

On Fri, 2021-02-05 at 14:27:24 UTC, Sascha Hauer wrote:
> On success chip->legacy.waitfunc() returns the NAND status byte, but on
> failure it returns a negative error code. This was never tested for and
> instead the return value was interpreted as NAND status without error
> checking. Add the missing error check.
> 
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>

Applied to https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git nand/next, thanks.

Miquel

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