Re: [PATCH 2.6.11-mm1] mips: more convert verify_area to access_ok

[Jesper Juhl]

On Sun, 6 Mar 2005, Yoichi Yuasa wrote:

> This patch converts verify_area to access_ok for include/asm-mips.
> 
Yeah, that's one of the few bits I had not done yet. Thank you for taking 
a look at that.

I don't believe your patch is correct though. See below for what I think 
is a better one.


> Yoichi
> 
> Signed-off-by: Yoichi Yuasa <yuasa@xxxxxxxxxxxxxx>
> 
> diff -urN -X dontdiff a-orig/include/asm-mips/uaccess.h a/include/asm-mips/uaccess.h
> --- a-orig/include/asm-mips/uaccess.h	Sat Mar  5 04:15:22 2005
> +++ a/include/asm-mips/uaccess.h	Sun Mar  6 15:51:02 2005
> @@ -254,13 +254,11 @@
>  ({									\
>  	__typeof__(*(ptr)) __gu_val = 0;				\
>  	long __gu_addr;							\
> -	long __gu_err;							\
> +	long __gu_err = -EFAULT;					\
>  									\
>  	might_sleep();							\
>  	__gu_addr = (long) (ptr);					\
> -	__gu_err = verify_area(VERIFY_READ, (void *) __gu_addr, size);	\
> -									\
> -	if (likely(!__gu_err)) {					\
> +	if (access_ok(VERIFY_READ, (void *) __gu_addr, size)) {		\
>  		switch (size) {						\
>  		case 1: __get_user_asm("lb", __gu_err); break;		\
>  		case 2: __get_user_asm("lh", __gu_err); break;		\

with this change, __gu_err will always be -EFAULT. With the original code 
it was either -EFAULT or 0 depending on the return value from verify_area. 
Same goes for the next hunk in your patch.

I believe a more correct patch would be this :

Signed-off-by: Jesper Juhl <juhl-lkml@xxxxxx>

diff -up linux-2.6.11-mm1-orig/include/asm-mips/uaccess.h linux-2.6.11-mm1/include/asm-mips/uaccess.h
--- linux-2.6.11-mm1-orig/include/asm-mips/uaccess.h	2005-03-05 00:39:40.000000000 +0100
+++ linux-2.6.11-mm1/include/asm-mips/uaccess.h	2005-03-07 00:49:24.000000000 +0100
@@ -258,7 +258,8 @@ struct __large_struct { unsigned long bu
 									\
 	might_sleep();							\
 	__gu_addr = (long) (ptr);					\
-	__gu_err = verify_area(VERIFY_READ, (void *) __gu_addr, size);	\
+	__gu_err = access_ok(VERIFY_READ, (void *) __gu_addr, size)	\
+				? 0 : -EFAULT;				\
 									\
 	if (likely(!__gu_err)) {					\
 		switch (size) {						\
@@ -353,7 +354,8 @@ extern void __get_user_unknown(void);
 	might_sleep();							\
 	__pu_val = (x);							\
 	__pu_addr = (long) (ptr);					\
-	__pu_err = verify_area(VERIFY_WRITE, (void *) __pu_addr, size);	\
+	__pu_err = access_ok(VERIFY_WRITE, (void *) __pu_addr, size)	\
+				? 0 : -EFAULT;				\
 									\
 	if (likely(!__pu_err)) {					\
 		switch (size) {						\




It preserves the exact behaviour of the original.


-- 
Jesper 


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/