Patchwork mtd: map: fixed bug in 64-bit systems

login
register
mail settings
Submitter Wang Haitao
Date Aug. 22, 2013, 11:32 a.m.
Message ID <OFF5F543EF.B1AF55C4-ON48257BCF.003EF309-48257BCF.003FC2C8@zte.com.cn>
Download mbox | patch
Permalink /patch/269021/
State New
Headers show

Comments

Wang Haitao - Aug. 22, 2013, 11:32 a.m.
Hardware:
	CPU:XLP832,the 64-bit OS
	NOR Flash:S29GL128S 128M
Software:
	Kernel:2.6.32.41
	Filesystem:JFFS2
When writing files, errors appear:
	Write len 182  but return retlen 180
	Write of 182 bytes at 0x072c815c failed. returned -5, retlen 180
	Write len 186  but return retlen 184
	Write of 186 bytes at 0x072caff4 failed. returned -5, retlen 184
These errors exist only in 64-bit systems,not in 32-bit systems. After analysis, we
found that the left shift operation is wrong in map_word_load_partial. For instance:
	unsigned char buf[3] ={0x9e,0x3a,0xea};
	map_bankwidth(map) is 4;
	for (i=0; i < 3; i++) {
		int bitpos;
		bitpos = (map_bankwidth(map)-1-i)*8;
		orig.x[0] &= ~(0xff << bitpos);
		orig.x[0] |= buf[i] << bitpos;
	}

The value of orig.x[0] is expected to be 0x9e3aeaff, but in this situation(64-bit
System) we'll get the wrong value of 0xffffffff9e3aeaff due to the 64-bit sign
extension:
buf[i] is defined as "unsigned char" and the left-shift operation will convert it
to the type of "signed int", so when left-shift buf[i] by 24 bits, the final result
will get the wrong value: 0xffffffff9e3aeaff.

If the left-shift bits are less than 24, then sign extension will not occur. Whereas
the bankwidth of the nor flash we used is 4, therefore this BUG emerges.

Signed-off-by:Pang Xunlei <pang.xunlei@zte.com.cn>
Signed-off-by: Zhang Yi <zhang.yi20@zte.com.cn>

Signed-off-by:Lu Zhongjun <lu.zhongjun@zte.com.cn>



                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                              
             Brian Norris <computersforpeace@gmail.com>                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                       
                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                              
             2013-08-21 16:57                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                              To 
                                                                                                                                                                                                                                                                                          Wang Haitao <wang.haitao1@zte.com.cn>                                                                                                                                                                                                                                                                                                                                                                                                                                               
                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                           cc 
                                                                                                                                                                                                                                                                                          linux-mtd@lists.infradead.org, artem.bityutskiy@linux.intel.com, dwmw2@infradead.org                                                                                                                                                                                                                                                                                                                                                                                                
                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                      Subject 
                                                                                                                                                                                                                                                                                          Re: [PATCH]mtd: map: fixed bug in 64-bit systems                                                                                                                                                                                                                                                                                                                                                                                                                                    
                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                              
                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                              
                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                              
                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                              
                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                              
                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                              




Hi Wang,

Can you please resend this patch with a few fixups?

The description below is good, but please wrap it to ~70 characters. It
is hard to read.

Also, you need to test your patch with scripts/checkpatch.pl both before
and after you email it -- all your whitespace is mangled (i.e., you use
spaces where there should be tabs).

See the kernel documentation:

  Documentation/SubmittingPatches
  Documentation/email-clients.txt

On Wed, Aug 07, 2013 at 03:34:20PM +0800, Wang Haitao wrote:
> Hardware:

>          CPU:XLP832,the 64-bit OS

>          NOR Flash:S29GL128S 128M

> Software:

>          Kernel:2.6.32.41

>          Filesystem:JFFS2

>

> When writing files, errors appear:

>          Write len 182  but return retlen 180

>          Write of 182 bytes at 0x072c815c failed. returned -5, retlen 180

>          Write len 186  but return retlen 184

>          Write of 186 bytes at 0x072caff4 failed. returned -5, retlen 184

> These errors exist only in 64-bit systems,not in 32-bit systems. After analysis, we found that the left shift operation is wrong in map_word_load_partial. For instance:

>          unsigned char buf[3] ={0x9e,0x3a,0xea};

>          map_bankwidth(map) is 4;

>

>          for (i=0; i < 3; i++) {

>                    int bitpos;

>                    bitpos = (map_bankwidth(map)-1-i)*8;

>                    orig.x[0] &= ~(0xff << bitpos);

>                    orig.x[0] |= buf[i] << bitpos;

>          }

>

> The value of orig.x[0] is expected to be 0x9e3aeaff, but in this situation(64-bit System) we'll get the wrong value of 0xffffffff9e3aeaff due to the 64-bit sign extension:

> buf[i] is defined as "unsigned char" and the left-shift operation will convert it to the type of "signed int", so when left-shift buf[i] by 24 bits, the final result will get the wrong value: 0xffffffff9e3aeaff.

>

> If the left-shift bits are less than 24, then sign extension will not occur. Whereas the bankwidth of the nor flash we used is 4, therefore this BUG emerges.

>

> Signed-off-by:Pang Xunlei <pang.xunlei@zte.com.cn>

> Signed-off-by: Zhang Yi <zhang.yi20@zte.com.cn>

> Signed-off-by:Lu Zhongjun <lu.zhongjun@zte.com.cn>

> Reviewed-by: Jiang Biao <jiang.biao2@zte.com.cn>

> Tested-by: Ma Chenggong <ma.chenggong@zte.com.cn>


[...]

Thanks,
Brian

Patch

diff --git a/include/linux/mtd/map.h b/include/linux/mtd/map.h

index aa30244..18ee717

--- a/include/linux/mtd/map.h

+++ b/include/linux/mtd/map.h

@@ -342,7 +342,7 @@  static inline map_word map_word_load_partial(struct map_info *map, map_word orig

 			bitpos = (map_bankwidth(map)-1-i)*8;
 #endif
 			orig.x[0] &= ~(0xff << bitpos);
-			orig.x[0] |= buf[i-start] << bitpos;

+			orig.x[0] |= (unsigned long)buf[i-start] << bitpos;

 		}
 	}
 	return orig;
@@ -361,7 +361,7 @@  static inline map_word map_word_ff(struct map_info *map)


 	if (map_bankwidth(map) < MAP_FF_LIMIT) {
 		int bw = 8 * map_bankwidth(map);
-		r.x[0] = (1 << bw) - 1;

+		r.x[0] = (1UL << bw) - 1;

 	} else {
 		for (i=0; i<map_words(map); i++)
 			r.x[i] = ~0UL;