[v2,07/14] mtd: rawnand: qcom: fix null pointer access for erased page detection

Message ID 1525350041-22995-8-git-send-email-absahu@codeaurora.org
State Superseded
Delegated to: Miquel Raynal
Headers show
Series
  • Update for QCOM NAND driver
Related show

Commit Message

Abhishek Sahu May 3, 2018, 12:20 p.m.
parse_read_errors can be called with only oob_buf in which case
data_buf will be NULL.  If data_buf is NULL, then don’t
treat this page as completely erased in case of ECC uncorrectable
error for RS ECC. For BCH ECC, the controller itself tells
regarding erased page in status register.

Signed-off-by: Abhishek Sahu <absahu@codeaurora.org>
---
* Changes from v1:

  1. Added more detail in commit message
  2. Added comment before each if/else

 drivers/mtd/nand/raw/qcom_nandc.c | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)

Comments

Miquel Raynal May 22, 2018, 7:16 a.m. | #1
Hi Abhishek,

On Thu,  3 May 2018 17:50:34 +0530, Abhishek Sahu
<absahu@codeaurora.org> wrote:

> parse_read_errors can be called with only oob_buf in which case
> data_buf will be NULL.  If data_buf is NULL, then don’t
> treat this page as completely erased in case of ECC uncorrectable
> error for RS ECC. For BCH ECC, the controller itself tells
> regarding erased page in status register.
> 
> Signed-off-by: Abhishek Sahu <absahu@codeaurora.org>
> ---
> * Changes from v1:
> 
>   1. Added more detail in commit message
>   2. Added comment before each if/else

Again, thanks for that.

> 
>  drivers/mtd/nand/raw/qcom_nandc.c | 18 +++++++++++++++---
>  1 file changed, 15 insertions(+), 3 deletions(-)

Acked-by: Miquel Raynal <miquel.raynal@bootlin.com>
Abhishek Sahu May 22, 2018, 2:11 p.m. | #2
On 2018-05-22 12:46, Miquel Raynal wrote:
> Hi Abhishek,
> 
> On Thu,  3 May 2018 17:50:34 +0530, Abhishek Sahu
> <absahu@codeaurora.org> wrote:
> 
>> parse_read_errors can be called with only oob_buf in which case
>> data_buf will be NULL.  If data_buf is NULL, then don’t
>> treat this page as completely erased in case of ECC uncorrectable
>> error for RS ECC. For BCH ECC, the controller itself tells
>> regarding erased page in status register.
>> 
>> Signed-off-by: Abhishek Sahu <absahu@codeaurora.org>
>> ---
>> * Changes from v1:
>> 
>>   1. Added more detail in commit message
>>   2. Added comment before each if/else
> 
> Again, thanks for that.
> 
>> 
>>  drivers/mtd/nand/raw/qcom_nandc.c | 18 +++++++++++++++---
>>  1 file changed, 15 insertions(+), 3 deletions(-)
> 
> Acked-by: Miquel Raynal <miquel.raynal@bootlin.com>

  Thanks Miquel for your review.

  Regards,
  Abhishek

Patch

diff --git a/drivers/mtd/nand/raw/qcom_nandc.c b/drivers/mtd/nand/raw/qcom_nandc.c
index e6a21598..fa38142 100644
--- a/drivers/mtd/nand/raw/qcom_nandc.c
+++ b/drivers/mtd/nand/raw/qcom_nandc.c
@@ -1613,13 +1613,24 @@  static int parse_read_errors(struct qcom_nand_host *host, u8 *data_buf,
 			int ret, ecclen, extraooblen;
 			void *eccbuf;
 
-			/* ignore erased codeword errors */
+			/*
+			 * For BCH ECC, ignore erased codeword errors, if
+			 * ERASED_CW bits are set.
+			 */
 			if (host->bch_enabled) {
 				erased = (erased_cw & ERASED_CW) == ERASED_CW ?
 					 true : false;
-			} else {
+			/*
+			 * For RS ECC, HW reports the erased CW by placing
+			 * special characters at certain offsets in the buffer.
+			 * These special characters will be valid only if
+			 * complete page is read i.e. data_buf is not NULL.
+			 */
+			} else if (data_buf) {
 				erased = erased_chunk_check_and_fixup(data_buf,
 								      data_len);
+			} else {
+				erased = false;
 			}
 
 			if (erased) {
@@ -1667,7 +1678,8 @@  static int parse_read_errors(struct qcom_nand_host *host, u8 *data_buf,
 			max_bitflips = max(max_bitflips, stat);
 		}
 
-		data_buf += data_len;
+		if (data_buf)
+			data_buf += data_len;
 		if (oob_buf)
 			oob_buf += oob_len + ecc->bytes;
 	}