* [PATCH] microblaze: fix get_user/put_user side-effects
@ 2010-05-06 21:38 Steven J. Magnani
2010-05-13 11:58 ` Michal Simek
0 siblings, 1 reply; 2+ messages in thread
From: Steven J. Magnani @ 2010-05-06 21:38 UTC (permalink / raw)
To: microblaze-uclinux; +Cc: monstr, linux-kernel, Steven J. Magnani
The Microblaze implementations of get_user() and (MMU) put_user() evaluate
the address argument more than once. This causes unexpected side-effects for
invocations that include increment operators, i.e. get_user(foo, bar++).
This patch also removes the distinction between MMU and noMMU put_user().
Without the patch:
$ echo 1234567890 > /proc/sys/kernel/core_pattern
$ cat /proc/sys/kernel/core_pattern
12345
Signed-off-by: Steven J. Magnani <steve@digidescorp.com>
---
diff -uprN a/arch/microblaze/include/asm/uaccess.h b/arch/microblaze/include/asm/uaccess.h
--- a/arch/microblaze/include/asm/uaccess.h 2010-04-27 10:14:33.000000000 -0500
+++ b/arch/microblaze/include/asm/uaccess.h 2010-05-06 16:21:02.000000000 -0500
@@ -182,6 +182,39 @@ extern long __user_bad(void);
* Returns zero on success, or -EFAULT on error.
* On error, the variable @x is set to zero.
*/
+#define get_user(x, ptr) \
+ __get_user_check((x), (ptr), sizeof(*(ptr)))
+
+#define __get_user_check(x, ptr, size) \
+({ \
+ unsigned long __gu_val = 0; \
+ const typeof(*(ptr)) __user *__gu_addr = (ptr); \
+ int __gu_err = 0; \
+ \
+ if (access_ok(VERIFY_READ, __gu_addr, size)) { \
+ switch (size) { \
+ case 1: \
+ __get_user_asm("lbu", __gu_addr, __gu_val, \
+ __gu_err); \
+ break; \
+ case 2: \
+ __get_user_asm("lhu", __gu_addr, __gu_val, \
+ __gu_err); \
+ break; \
+ case 4: \
+ __get_user_asm("lw", __gu_addr, __gu_val, \
+ __gu_err); \
+ break; \
+ default: \
+ __gu_err = __user_bad(); \
+ break; \
+ } \
+ } else { \
+ __gu_err = -EFAULT; \
+ } \
+ x = (typeof(*(ptr)))__gu_val; \
+ __gu_err; \
+})
#define __get_user(x, ptr) \
({ \
@@ -206,12 +239,6 @@ extern long __user_bad(void);
})
-#define get_user(x, ptr) \
-({ \
- access_ok(VERIFY_READ, (ptr), sizeof(*(ptr))) \
- ? __get_user((x), (ptr)) : -EFAULT; \
-})
-
#define __put_user_asm(insn, __gu_ptr, __gu_val, __gu_err) \
({ \
__asm__ __volatile__ ( \
@@ -266,6 +293,42 @@ extern long __user_bad(void);
*
* Returns zero on success, or -EFAULT on error.
*/
+#define put_user(x, ptr) \
+ __put_user_check((x), (ptr), sizeof(*(ptr)))
+
+#define __put_user_check(x, ptr, size) \
+({ \
+ typeof(*(ptr)) __pu_val; \
+ typeof(*(ptr)) __user *__pu_addr = (ptr); \
+ int __pu_err = 0; \
+ \
+ __pu_val = (x); \
+ if (access_ok(VERIFY_WRITE, __pu_addr, size)) { \
+ switch (size) { \
+ case 1: \
+ __put_user_asm("sb", __pu_addr, __pu_val, \
+ __pu_err); \
+ break; \
+ case 2: \
+ __put_user_asm("sh", __pu_addr, __pu_val, \
+ __pu_err); \
+ break; \
+ case 4: \
+ __put_user_asm("sw", __pu_addr, __pu_val, \
+ __pu_err); \
+ break; \
+ case 8: \
+ __put_user_asm_8(__pu_addr, __pu_val, __pu_err);\
+ break; \
+ default: \
+ __pu_err = __user_bad(); \
+ break; \
+ } \
+ } else { \
+ __pu_err = -EFAULT; \
+ } \
+ __pu_err; \
+})
#define __put_user(x, ptr) \
({ \
@@ -290,18 +353,6 @@ extern long __user_bad(void);
__gu_err; \
})
-#ifndef CONFIG_MMU
-
-#define put_user(x, ptr) __put_user((x), (ptr))
-
-#else /* CONFIG_MMU */
-
-#define put_user(x, ptr) \
-({ \
- access_ok(VERIFY_WRITE, (ptr), sizeof(*(ptr))) \
- ? __put_user((x), (ptr)) : -EFAULT; \
-})
-#endif /* CONFIG_MMU */
/* copy_to_from_user */
#define __copy_from_user(to, from, n) \
^ permalink raw reply [flat|nested] 2+ messages in thread* Re: [PATCH] microblaze: fix get_user/put_user side-effects
2010-05-06 21:38 [PATCH] microblaze: fix get_user/put_user side-effects Steven J. Magnani
@ 2010-05-13 11:58 ` Michal Simek
0 siblings, 0 replies; 2+ messages in thread
From: Michal Simek @ 2010-05-13 11:58 UTC (permalink / raw)
To: Steven J. Magnani; +Cc: microblaze-uclinux, linux-kernel
Steven J. Magnani wrote:
> The Microblaze implementations of get_user() and (MMU) put_user() evaluate
> the address argument more than once. This causes unexpected side-effects for
> invocations that include increment operators, i.e. get_user(foo, bar++).
>
> This patch also removes the distinction between MMU and noMMU put_user().
>
> Without the patch:
> $ echo 1234567890 > /proc/sys/kernel/core_pattern
> $ cat /proc/sys/kernel/core_pattern
> 12345
Applied,
I believe that in future we could use inline function which should solve
this problem too.
Thanks,
Michal
>
> Signed-off-by: Steven J. Magnani <steve@digidescorp.com>
> ---
> diff -uprN a/arch/microblaze/include/asm/uaccess.h b/arch/microblaze/include/asm/uaccess.h
> --- a/arch/microblaze/include/asm/uaccess.h 2010-04-27 10:14:33.000000000 -0500
> +++ b/arch/microblaze/include/asm/uaccess.h 2010-05-06 16:21:02.000000000 -0500
> @@ -182,6 +182,39 @@ extern long __user_bad(void);
> * Returns zero on success, or -EFAULT on error.
> * On error, the variable @x is set to zero.
> */
> +#define get_user(x, ptr) \
> + __get_user_check((x), (ptr), sizeof(*(ptr)))
> +
> +#define __get_user_check(x, ptr, size) \
> +({ \
> + unsigned long __gu_val = 0; \
> + const typeof(*(ptr)) __user *__gu_addr = (ptr); \
> + int __gu_err = 0; \
> + \
> + if (access_ok(VERIFY_READ, __gu_addr, size)) { \
> + switch (size) { \
> + case 1: \
> + __get_user_asm("lbu", __gu_addr, __gu_val, \
> + __gu_err); \
> + break; \
> + case 2: \
> + __get_user_asm("lhu", __gu_addr, __gu_val, \
> + __gu_err); \
> + break; \
> + case 4: \
> + __get_user_asm("lw", __gu_addr, __gu_val, \
> + __gu_err); \
> + break; \
> + default: \
> + __gu_err = __user_bad(); \
> + break; \
> + } \
> + } else { \
> + __gu_err = -EFAULT; \
> + } \
> + x = (typeof(*(ptr)))__gu_val; \
> + __gu_err; \
> +})
>
> #define __get_user(x, ptr) \
> ({ \
> @@ -206,12 +239,6 @@ extern long __user_bad(void);
> })
>
>
> -#define get_user(x, ptr) \
> -({ \
> - access_ok(VERIFY_READ, (ptr), sizeof(*(ptr))) \
> - ? __get_user((x), (ptr)) : -EFAULT; \
> -})
> -
> #define __put_user_asm(insn, __gu_ptr, __gu_val, __gu_err) \
> ({ \
> __asm__ __volatile__ ( \
> @@ -266,6 +293,42 @@ extern long __user_bad(void);
> *
> * Returns zero on success, or -EFAULT on error.
> */
> +#define put_user(x, ptr) \
> + __put_user_check((x), (ptr), sizeof(*(ptr)))
> +
> +#define __put_user_check(x, ptr, size) \
> +({ \
> + typeof(*(ptr)) __pu_val; \
> + typeof(*(ptr)) __user *__pu_addr = (ptr); \
> + int __pu_err = 0; \
> + \
> + __pu_val = (x); \
> + if (access_ok(VERIFY_WRITE, __pu_addr, size)) { \
> + switch (size) { \
> + case 1: \
> + __put_user_asm("sb", __pu_addr, __pu_val, \
> + __pu_err); \
> + break; \
> + case 2: \
> + __put_user_asm("sh", __pu_addr, __pu_val, \
> + __pu_err); \
> + break; \
> + case 4: \
> + __put_user_asm("sw", __pu_addr, __pu_val, \
> + __pu_err); \
> + break; \
> + case 8: \
> + __put_user_asm_8(__pu_addr, __pu_val, __pu_err);\
> + break; \
> + default: \
> + __pu_err = __user_bad(); \
> + break; \
> + } \
> + } else { \
> + __pu_err = -EFAULT; \
> + } \
> + __pu_err; \
> +})
>
> #define __put_user(x, ptr) \
> ({ \
> @@ -290,18 +353,6 @@ extern long __user_bad(void);
> __gu_err; \
> })
>
> -#ifndef CONFIG_MMU
> -
> -#define put_user(x, ptr) __put_user((x), (ptr))
> -
> -#else /* CONFIG_MMU */
> -
> -#define put_user(x, ptr) \
> -({ \
> - access_ok(VERIFY_WRITE, (ptr), sizeof(*(ptr))) \
> - ? __put_user((x), (ptr)) : -EFAULT; \
> -})
> -#endif /* CONFIG_MMU */
>
> /* copy_to_from_user */
> #define __copy_from_user(to, from, n) \
>
--
Michal Simek, Ing. (M.Eng)
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel 2.6 Microblaze Linux - http://www.monstr.eu/fdt/
Microblaze U-BOOT custodian
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2010-05-13 11:59 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-05-06 21:38 [PATCH] microblaze: fix get_user/put_user side-effects Steven J. Magnani
2010-05-13 11:58 ` Michal Simek
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox