A userland read of more than PAGE_SIZE bytes from /dev/zero results in
(a) not all of the bytes returned being zero, and
(b) memory corruption due to zeroing of bytes beyond the user buffer.
This is caused by improper constraints on the assembly __clear_user function.
The constrints don't indicate to the compiler that the pointer argument is
modified. Since the function is inline, this results in double-incrementing
of the pointer when __clear_user() is invoked through a multi-page read() of
/dev/zero.
Signed-off-by: Steven J. Magnani <[email protected]>
---
diff -uprN a/arch/microblaze/include/asm/uaccess.h b/arch/microblaze/include/asm/uaccess.h
--- a/arch/microblaze/include/asm/uaccess.h 2011-02-10 11:51:52.319226513 -0600
+++ b/arch/microblaze/include/asm/uaccess.h 2011-02-10 11:53:01.064857179 -0600
@@ -120,16 +120,16 @@ static inline unsigned long __must_check
{
/* normal memset with two words to __ex_table */
__asm__ __volatile__ ( \
- "1: sb r0, %2, r0;" \
+ "1: sb r0, %1, r0;" \
" addik %0, %0, -1;" \
" bneid %0, 1b;" \
- " addik %2, %2, 1;" \
+ " addik %1, %1, 1;" \
"2: " \
__EX_TABLE_SECTION \
".word 1b,2b;" \
".previous;" \
- : "=r"(n) \
- : "0"(n), "r"(to)
+ : "=r"(n), "=r"(to) \
+ : "0"(n), "1"(to)
);
return n;
}
Steven J. Magnani wrote:
> A userland read of more than PAGE_SIZE bytes from /dev/zero results in
> (a) not all of the bytes returned being zero, and
> (b) memory corruption due to zeroing of bytes beyond the user buffer.
>
> This is caused by improper constraints on the assembly __clear_user function.
> The constrints don't indicate to the compiler that the pointer argument is
> modified. Since the function is inline, this results in double-incrementing
> of the pointer when __clear_user() is invoked through a multi-page read() of
> /dev/zero.
>
> Signed-off-by: Steven J. Magnani <[email protected]>
I wasn't able to reproduce this issue on MMU and noMMU systems (log below).
I think it is toolchain issue.
Anyway it will be the best not to increment "to" at all and use "n" as offset.
This could also speed it up this function.
There is also possibility to add sb to delay slot but it works from Microblaze 5.00.a and later.
I will add this patch to my next branch and will be good to get some statistic and then to write
some optimization for it.
Acked-by: Michal Simek <[email protected]>
Thanks,
Michal
My log:
# dd if=/dev/zero of=/tmp/1 bs=64k count=1
1+0 records in
1+0 records out
~ # hexdump /tmp/1
0000000 0000 0000 0000 0000 0000 0000 0000 0000
*
0010000
~ # md5sum /tmp/1
fcd6bcb56c1689fcef28b57c22475bad /tmp/1
~ # cat /proc/cpuinfo
CPU-Family: MicroBlaze
FPGA-Arch: spartan6
CPU-Ver: 7.30.a, big endian
CPU-MHz: 90.00
BogoMips: 44.54
HW:
Shift: yes
MSR: yes
PCMP: yes
DIV: no
MMU: 3
MUL: v2
FPU: no
Exc: ill
Icache: 16kB line length: 16B
Dcache: 16kB line length: 16B
write-through
HW-Debug: yes
PVR-USR1: 00
PVR-USR2: 00000000
Page size: 16384
~ #
--
Michal Simek, Ing. (M.Eng)
w: http://www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel 2.6 Microblaze Linux - http://www.monstr.eu/fdt/
Microblaze U-BOOT custodian