doc2000: Fix uninitialized variable in doc_ecc_decode()

RFC/PATCH Mark Ware 2 participants 7 messages May 27, 2010 – Jun 29, 2010

Subsystems (2)

Memory technology devices (MTD) NAND flash

lore

Share & tools

lore

Mark Ware

author

2010-05-27 01:45 UTC #0 lore raw

The variable 'syn' was being used uninitialized. Also fixed incorrect use of syn[] vs s[].

Tested on powerpc board with 64MB DOC2000.
---

I am porting from a 2.4.18 kernel to 2.6.32, and I saw random media header mismatches causing a failure to detect the DOC device partitions. Tracing through, I saw this variable being used uninitialized and I suspect incorrectly also.

I do not really understand how the ecc/syndrome code works, so I do not know if this patch is the correct solution, but it did make my problem go away...

CC Thomas Gleixner as I believe he may have written this function initially.

 drivers/mtd/nand/diskonchip.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

drivers/mtd/nand/diskonchip.c +3 -2

index a5bf9ff..7da2321 100644
@@ -145,6 +145,7 @@ static int doc_ecc_decode(struct rs_control *rs, uint8_t *data, uint8_t *ecc)
 	uint8_t parity;
 	uint16_t ds[4], s[5], tmp, errval[8], syn[4];
 
+	memset(syn, 0, sizeof(syn));
 	/* Convert the ecc bytes into words */
 	ds[0] = ((ecc[4] & 0xff) >> 0) | ((ecc[5] & 0x03) << 8);
 	ds[1] = ((ecc[5] & 0xfc) >> 2) | ((ecc[2] & 0x0f) << 6);

@@ -168,9 +169,9 @@ static int doc_ecc_decode(struct rs_control *rs, uint8_t *data, uint8_t *ecc)
 			s[i] ^= rs->alpha_to[rs_modnn(rs, tmp + (FCR + i) * j)];
 	}
 
-	/* Calc s[i] = s[i] / alpha^(v + i) */
+	/* Calc syn[i] = s[i] / alpha^(v + i) */
 	for (i = 0; i < NROOTS; i++) {
-		if (syn[i])
+		if (s[i])
 			syn[i] = rs_modnn(rs, rs->index_of[s[i]] + (NN - FCR - i));
 	}
 	/* Call the decoder library */

Signature · 1 line

1.5.6.5

Mark Ware

author

2010-06-17 04:44 UTC #1 lore raw

No comments?
I know DOC2000 is pretty old now, but somebody must remember them!

Regards,
Mark

Quoted 44 lines · Mark Ware

The variable 'syn' was being used uninitialized. Also fixed incorrect use of syn[] vs s[].
Tested on powerpc board with 64MB DOC2000.
---
I am porting from a 2.4.18 kernel to 2.6.32, and I saw random media header mismatches causing a failure to detect the DOC device partitions. Tracing through, I saw this variable being used uninitialized and I suspect incorrectly also.
I do not really understand how the ecc/syndrome code works, so I do not know if this patch is the correct solution, but it did make my problem go away...
CC Thomas Gleixner as I believe he may have written this function initially.
 drivers/mtd/nand/diskonchip.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)
drivers/mtd/nand/diskonchip.c +3 -2
index a5bf9ff..7da2321 100644
@@ -145,6 +145,7 @@ static int doc_ecc_decode(struct rs_control *rs, uint8_t *data, uint8_t *ecc)
 	uint8_t parity;
 	uint16_t ds[4], s[5], tmp, errval[8], syn[4];
 
+	memset(syn, 0, sizeof(syn));
 	/* Convert the ecc bytes into words */
 	ds[0] = ((ecc[4] & 0xff) >> 0) | ((ecc[5] & 0x03) << 8);
 	ds[1] = ((ecc[5] & 0xfc) >> 2) | ((ecc[2] & 0x0f) << 6);
@@ -168,9 +169,9 @@ static int doc_ecc_decode(struct rs_control *rs, uint8_t *data, uint8_t *ecc)
 			s[i] ^= rs->alpha_to[rs_modnn(rs, tmp + (FCR + i) * j)];
 	}
 
-	/* Calc s[i] = s[i] / alpha^(v + i) */
+	/* Calc syn[i] = s[i] / alpha^(v + i) */
 	for (i = 0; i < NROOTS; i++) {
-		if (syn[i])
+		if (s[i])
 			syn[i] = rs_modnn(rs, rs->index_of[s[i]] + (NN - FCR - i));
 	}
 	/* Call the decoder library */

Artem Bityutskiy

2010-06-29 03:46 UTC #2 lore raw

In reply to Mark Ware

Quoted 25 lines · Mark Ware

The variable 'syn' was being used uninitialized. Also fixed incorrect use of syn[] vs s[].
Tested on powerpc board with 64MB DOC2000.
---
I am porting from a 2.4.18 kernel to 2.6.32, and I saw random media header mismatches causing a failure to detect the DOC device partitions. Tracing through, I saw this variable being used uninitialized and I suspect incorrectly also.
I do not really understand how the ecc/syndrome code works, so I do not know if this patch is the correct solution, but it did make my problem go away...
CC Thomas Gleixner as I believe he may have written this function initially.
 drivers/mtd/nand/diskonchip.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)
drivers/mtd/nand/diskonchip.c +0 -0
index a5bf9ff..7da2321 100644

drivers/mtd/nand/diskonchip.c static int doc_ecc_decode(struct rs_control *rs, uint8_t *data, uint8_t *ecc) · line 148

Quoted 4 lines

drivers/mtd/nand/diskonchip.c +1 -0

@@ -145,6 +145,7 @@ static int doc_ecc_decode(struct rs_control *rs, uint8_t *data, uint8_t *ecc)
 	uint8_t parity;
 	uint16_t ds[4], s[5], tmp, errval[8], syn[4];
 
+	memset(syn, 0, sizeof(syn));

I also do not know the math of this stuff, but this change is not needed ...

Quoted 4 lines

drivers/mtd/nand/diskonchip.c +0 -0

 	/* Convert the ecc bytes into words */
 	ds[0] = ((ecc[4] & 0xff) >> 0) | ((ecc[5] & 0x03) << 8);
 	ds[1] = ((ecc[5] & 0xfc) >> 2) | ((ecc[2] & 0x0f) << 6);

drivers/mtd/nand/diskonchip.c static int doc_ecc_decode(struct rs_control *rs, uint8_t *data, uint8_t *ecc) · line 176

Quoted 10 lines

drivers/mtd/nand/diskonchip.c +2 -2

@@ -168,9 +169,9 @@ static int doc_ecc_decode(struct rs_control *rs, uint8_t *data, uint8_t *ecc)
 			s[i] ^= rs->alpha_to[rs_modnn(rs, tmp + (FCR + i) * j)];
 	}
 
-	/* Calc s[i] = s[i] / alpha^(v + i) */
+	/* Calc syn[i] = s[i] / alpha^(v + i) */
 	for (i = 0; i < NROOTS; i++) {
-		if (syn[i])
+		if (s[i])
 			syn[i] = rs_modnn(rs, rs->index_of[s[i]] + (NN - FCR - i));
 	}

... after this change, right?

Signature · 2 lines

Best Regards,
Artem Bityutskiy (Артём Битюцкий)

Artem Bityutskiy

2010-06-29 03:58 UTC #3 lore raw

Quoted 29 lines · Artem Bityutskiy

Quoted 28 lines · Mark Ware
The variable 'syn' was being used uninitialized. Also fixed incorrect use of syn[] vs s[].
Tested on powerpc board with 64MB DOC2000.
---
I am porting from a 2.4.18 kernel to 2.6.32, and I saw random media header mismatches causing a failure to detect the DOC device partitions. Tracing through, I saw this variable being used uninitialized and I suspect incorrectly also.
I do not really understand how the ecc/syndrome code works, so I do not know if this patch is the correct solution, but it did make my problem go away...
CC Thomas Gleixner as I believe he may have written this function initially.
 drivers/mtd/nand/diskonchip.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)
drivers/mtd/nand/diskonchip.c +0 -0
index a5bf9ff..7da2321 100644
@@ -145,6 +145,7 @@ static int doc_ecc_decode(struct rs_control *rs, uint8_t *data, uint8_t *ecc)
 	uint8_t parity;
 	uint16_t ds[4], s[5], tmp, errval[8], syn[4];
 

Quoted 4 lines

drivers/mtd/nand/diskonchip.c static int doc_ecc_decode(struct rs_control *rs, uint8_t *data, uint8_t *ecc) · line 148
Quoted 1 line
drivers/mtd/nand/diskonchip.c +1 -0
+	memset(syn, 0, sizeof(syn));
I also do not know the math of this stuff, but this change is not needed ...

Sorry, ignore this, I'll put this patch to my l2-mtd-2.6 tree and let dwmw2 decide whether it is good or not :-)

Signature · 2 lines

Best Regards,
Artem Bityutskiy (Артём Битюцкий)

Artem Bityutskiy

2010-06-29 06:33 UTC #4 lore raw

In reply to Mark Ware

Quoted 11 lines · Mark Ware

The variable 'syn' was being used uninitialized. Also fixed incorrect use of syn[] vs s[].
Tested on powerpc board with 64MB DOC2000.
---
I am porting from a 2.4.18 kernel to 2.6.32, and I saw random media header mismatches causing a failure to detect the DOC device partitions. Tracing through, I saw this variable being used uninitialized and I suspect incorrectly also.

Quoted 5 lines

I do not really understand how the ecc/syndrome code works, so I do not know if this patch is the correct solution, but it did make my problem go away...
CC Thomas Gleixner as I believe he may have written this function initially.

Please, always add Signed-off-by for kernel patches. I added it for you.

Signature · 2 lines

Best Regards,
Artem Bityutskiy (Артём Битюцкий)

Mark Ware

author

2010-06-29 06:47 UTC #5 lore raw

Quoted 12 lines · Artem Bityutskiy

Quoted 11 lines · Mark Ware
The variable 'syn' was being used uninitialized. Also fixed incorrect use of syn[] vs s[].
Tested on powerpc board with 64MB DOC2000.
---
I am porting from a 2.4.18 kernel to 2.6.32, and I saw random media header mismatches causing a failure to detect the DOC device partitions. Tracing through, I saw this variable being used uninitialized and I suspect incorrectly also.

Quoted 8 lines

Quoted 5 lines
I do not really understand how the ecc/syndrome code works, so I do not know if this patch is the correct solution, but it did make my problem go away...
CC Thomas Gleixner as I believe he may have written this function initially.
Please, always add Signed-off-by for kernel patches. I added it for you.

Thanks. I guess I expected to be told that I was smoking something and that my DOC error disappearing was unrelated...

Mark

Artem Bityutskiy

2010-06-29 06:53 UTC #6 lore raw

Quoted 17 lines · Mark Ware

Quoted 16 lines · Artem Bityutskiy
Quoted 15 lines · Mark Ware
The variable 'syn' was being used uninitialized. Also fixed incorrect use of syn[] vs s[].
Tested on powerpc board with 64MB DOC2000.
---
I am porting from a 2.4.18 kernel to 2.6.32, and I saw random media header mismatches causing a failure to detect the DOC device partitions. Tracing through, I saw this variable being used uninitialized and I suspect incorrectly also.
I do not really understand how the ecc/syndrome code works, so I do not know if this patch is the correct solution, but it did make my problem go away...

Quoted 7 lines

Quoted 4 lines
Quoted 1 line
CC Thomas Gleixner as I believe he may have written this function initially.
Please, always add Signed-off-by for kernel patches. I added it for you.
Thanks. I guess I expected to be told that I was smoking something and that my DOC error disappearing was unrelated...

Note, I did not really review your patch - the MTD maintainer will take a look at it later, probably.

Signature · 2 lines

Best Regards,
Artem Bityutskiy (Артём Битюцкий)