[RFC/PATCH] doc2000: Fix uninitialized variable in doc_ecc_decode()

[RFC/PATCH] doc2000: Fix uninitialized variable in doc_ecc_decode()

[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(-)

diff --git a/drivers/mtd/nand/diskonchip.c b/drivers/mtd/nand/diskonchip.c
index a5bf9ff..7da2321 100644
--- a/drivers/mtd/nand/diskonchip.c
+++ b/drivers/mtd/nand/diskonchip.c
@@ -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 */
-- 
1.5.6.5

Re: [RFC/PATCH] doc2000: Fix uninitialized variable in doc_ecc_decode()

[Mark Ware]

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

Regards,
Mark

Mark Ware wrote:
> 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(-)
> 
> diff --git a/drivers/mtd/nand/diskonchip.c b/drivers/mtd/nand/diskonchip.c
> index a5bf9ff..7da2321 100644
> --- a/drivers/mtd/nand/diskonchip.c
> +++ b/drivers/mtd/nand/diskonchip.c
> @@ -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 */

Re: [RFC/PATCH] doc2000: Fix uninitialized variable in doc_ecc_decode()

[Artem Bityutskiy]

On Thu, 2010-05-27 at 11:45 +1000, Mark Ware wrote:
> 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(-)
> 
> diff --git a/drivers/mtd/nand/diskonchip.c b/drivers/mtd/nand/diskonchip.c
> index a5bf9ff..7da2321 100644
> --- a/drivers/mtd/nand/diskonchip.c
> +++ b/drivers/mtd/nand/diskonchip.c
> @@ -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 ...

>  	/* 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));
>  	}

... after this change, right?

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

Re: [RFC/PATCH] doc2000: Fix uninitialized variable in doc_ecc_decode()

[Artem Bityutskiy]

On Tue, 2010-06-29 at 06:46 +0300, Artem Bityutskiy wrote:
> On Thu, 2010-05-27 at 11:45 +1000, Mark Ware wrote:
> > 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(-)
> > 
> > diff --git a/drivers/mtd/nand/diskonchip.c b/drivers/mtd/nand/diskonchip.c
> > index a5bf9ff..7da2321 100644
> > --- a/drivers/mtd/nand/diskonchip.c
> > +++ b/drivers/mtd/nand/diskonchip.c
> > @@ -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 ...

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 :-)
-- 
Best Regards,
Artem Bityutskiy (Артём Битюцкий)

Re: [RFC/PATCH] doc2000: Fix uninitialized variable in doc_ecc_decode()

[Artem Bityutskiy]

On Thu, 2010-05-27 at 11:45 +1000, Mark Ware wrote:
> 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.

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

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

Re: [RFC/PATCH] doc2000: Fix uninitialized variable in doc_ecc_decode()

[Mark Ware]

On 29/06/10 16:29, Artem Bityutskiy wrote:
> On Thu, 2010-05-27 at 11:45 +1000, Mark Ware wrote:
>> 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.
> 
> 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

Re: [RFC/PATCH] doc2000: Fix uninitialized variable in doc_ecc_decode()

[Artem Bityutskiy]

On Tue, 2010-06-29 at 16:47 +1000, Mark Ware wrote:
> On 29/06/10 16:29, Artem Bityutskiy wrote:
> > On Thu, 2010-05-27 at 11:45 +1000, Mark Ware wrote:
> >> 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.
> > 
> > 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.

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