* [PATCH]mtd: map: fixed bug in 64-bit systems
@ 2013-08-07 7:34 Wang Haitao
0 siblings, 0 replies; 5+ messages in thread
From: Wang Haitao @ 2013-08-07 7:34 UTC (permalink / raw)
To: linux-mtd; +Cc: artem.bityutskiy, dwmw2, wang.haitao1
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>
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;
--------------------------------------------------------
ZTE Information Security Notice: The information contained in this mail (and any attachment transmitted herewith) is privileged and confidential and is intended for the exclusive use of the addressee(s). If you are not an intended recipient, any disclosure, reproduction, distribution or other dissemination or use of the information contained is strictly prohibited. If you have received this mail in error, please delete it and notify us immediately.
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH]mtd: map: fixed bug in 64-bit systems
[not found] <5201f818.42ceb40a.21ae.ffffaa90SMTPIN_ADDED_BROKEN@mx.google.com>
@ 2013-08-21 8:57 ` Brian Norris
2013-08-22 11:32 ` Wang Haitao
2013-08-23 2:24 ` Wang Haitao
0 siblings, 2 replies; 5+ messages in thread
From: Brian Norris @ 2013-08-21 8:57 UTC (permalink / raw)
To: Wang Haitao; +Cc: artem.bityutskiy, linux-mtd, dwmw2
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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH]mtd: map: fixed bug in 64-bit systems
2013-08-21 8:57 ` [PATCH]mtd: map: fixed bug in 64-bit systems Brian Norris
@ 2013-08-22 11:32 ` Wang Haitao
2013-08-23 2:24 ` Wang Haitao
1 sibling, 0 replies; 5+ messages in thread
From: Wang Haitao @ 2013-08-22 11:32 UTC (permalink / raw)
To: Brian Norris; +Cc: artem.bityutskiy, dwmw2, linux-mtd
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>
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;
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
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH]mtd: map: fixed bug in 64-bit systems
2013-08-21 8:57 ` [PATCH]mtd: map: fixed bug in 64-bit systems Brian Norris
2013-08-22 11:32 ` Wang Haitao
@ 2013-08-23 2:24 ` Wang Haitao
2013-10-23 1:43 ` Brian Norris
1 sibling, 1 reply; 5+ messages in thread
From: Wang Haitao @ 2013-08-23 2:24 UTC (permalink / raw)
To: Brian Norris; +Cc: artem.bityutskiy, dwmw2, linux-mtd
Thanks for your advice.
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>
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;
Brian Norris <computersforpeace@gmail.com> 写于 2013-08-21 16:57:58:
> 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
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH]mtd: map: fixed bug in 64-bit systems
2013-08-23 2:24 ` Wang Haitao
@ 2013-10-23 1:43 ` Brian Norris
0 siblings, 0 replies; 5+ messages in thread
From: Brian Norris @ 2013-10-23 1:43 UTC (permalink / raw)
To: Wang Haitao; +Cc: artem.bityutskiy, dwmw2, linux-mtd
On Fri, Aug 23, 2013 at 10:24:07AM +0800, Wang Haitao wrote:
> Thanks for your advice.
>
> 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>
Well, this still wasn't formatted correctly, but it is a good fix. I
fixed it up, added the Cc: <stable@vger.kernel.org> and applied it to
l2-mtd.git. Thanks!
Brian
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2013-10-23 1:43 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <5201f818.42ceb40a.21ae.ffffaa90SMTPIN_ADDED_BROKEN@mx.google.com>
2013-08-21 8:57 ` [PATCH]mtd: map: fixed bug in 64-bit systems Brian Norris
2013-08-22 11:32 ` Wang Haitao
2013-08-23 2:24 ` Wang Haitao
2013-10-23 1:43 ` Brian Norris
2013-08-07 7:34 Wang Haitao
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox