[patch][resend] convert a remaining verify_area to access_ok (was: Re: [PATCH 2.6.11-mm1] mips: more convert verify_area to access_ok) (fwd)

[patch][resend] convert a remaining verify_area to access_ok (was: Re: [PATCH 2.6.11-mm1] mips: more convert verify_area to access_ok) (fwd)

[Jesper Juhl]

Hi Andrew,

Around 2.6.11-mm1 Yoichi Yuasa found a user of verify_area that I had 
missed when converting everything to access_ok. The patch below still 
applies cleanly to 2.6.11-mm4.
Please apply (unless of course you already picked it up back then and 
have it in a queue somewhere :) .

-- 
Jesper Juhl


---------- Forwarded message ----------
Date: Mon, 7 Mar 2005 00:55:30 +0100 (CET)
From: Jesper Juhl <juhl-lkml@dif.dk>
To: Yoichi Yuasa <yuasa@hh.iij4u.or.jp>
Cc: Andrew Morton <akpm@osdl.org>, linux-kernel <linux-kernel@vger.kernel.org>,
    Ralf Baechle <ralf@linux-mips.org>
Subject: Re: [PATCH 2.6.11-mm1] mips: more convert verify_area to access_ok

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@hh.iij4u.or.jp>
> 
> 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@dif.dk>

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@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [patch][resend] convert a remaining verify_area to access_ok (was: Re: [PATCH 2.6.11-mm1] mips: more convert verify_area to access_ok) (fwd)

[Andrew Morton]

Jesper Juhl <juhl-lkml@dif.dk> wrote:
>
> Around 2.6.11-mm1 Yoichi Yuasa found a user of verify_area that I had 
>  missed when converting everything to access_ok. The patch below still 
>  applies cleanly to 2.6.11-mm4.
>  Please apply (unless of course you already picked it up back then and 
>  have it in a queue somewhere :) .

That's tricky stuff you're playing with, so I'd prefer it came in via Ralf.
However I can queue it up locally so it doesn't get forgotten.

Ralf must have another two megabyte patch buffered up by now, btw?

Re: [patch][resend] convert a remaining verify_area to access_ok (was: Re: [PATCH 2.6.11-mm1] mips: more convert verify_area to access_ok) (fwd)

[Jesper Juhl]

On Wed, 16 Mar 2005, Andrew Morton wrote:

> Jesper Juhl <juhl-lkml@dif.dk> wrote:
> >
> > Around 2.6.11-mm1 Yoichi Yuasa found a user of verify_area that I had 
> >  missed when converting everything to access_ok. The patch below still 
> >  applies cleanly to 2.6.11-mm4.
> >  Please apply (unless of course you already picked it up back then and 
> >  have it in a queue somewhere :) .
> 
> That's tricky stuff you're playing with, so I'd prefer it came in via Ralf.
> However I can queue it up locally so it doesn't get forgotten.
> 
Perfectly fine by me.

-- 
Jesper

Re: [patch][resend] convert a remaining verify_area to access_ok (was: Re: [PATCH 2.6.11-mm1] mips: more convert verify_area to access_ok) (fwd)

[Ralf Baechle]

On Wed, Mar 16, 2005 at 02:55:24PM -0800, Andrew Morton wrote:

> Jesper Juhl <juhl-lkml@dif.dk> wrote:
> >
> > Around 2.6.11-mm1 Yoichi Yuasa found a user of verify_area that I had 
> >  missed when converting everything to access_ok. The patch below still 
> >  applies cleanly to 2.6.11-mm4.
> >  Please apply (unless of course you already picked it up back then and 
> >  have it in a queue somewhere :) .
> 
> That's tricky stuff you're playing with, so I'd prefer it came in via Ralf.
> However I can queue it up locally so it doesn't get forgotten.
> 
> Ralf must have another two megabyte patch buffered up by now, btw?

Quite a bit less and much of the diff are patches that must be somewhere
in Jeff's network driver queue.  But yes, hint taken, you'll get your
patch easter egg ;-0

  Ralf

Re: [patch][resend] convert a remaining verify_area to access_ok (was: Re: [PATCH 2.6.11-mm1] mips: more convert verify_area to access_ok) (fwd)

[Ralf Baechle]

On Wed, Mar 16, 2005 at 10:35:09PM +0100, Jesper Juhl wrote:

> Around 2.6.11-mm1 Yoichi Yuasa found a user of verify_area that I had 
> missed when converting everything to access_ok. The patch below still 
> applies cleanly to 2.6.11-mm4.
> Please apply (unless of course you already picked it up back then and 
> have it in a queue somewhere :) .

Oh gosh, you actually converted the whole IRIX compatibility mess even,
amazing stomach you have :-)  I only noticed that when I just looked at
Linus' tree - after buring a few hours into cleaning those files myself -
mine are now almost free of sparse warnings.

The last instance of verify_area() in the MIPS code is now the definition
itself.

  Ralf

Re: [patch][resend] convert a remaining verify_area to access_ok (was: Re: [PATCH 2.6.11-mm1] mips: more convert verify_area to access_ok) (fwd)

[Ralf Baechle]

On Wed, Mar 16, 2005 at 02:55:24PM -0800, Andrew Morton wrote:

> That's tricky stuff you're playing with, so I'd prefer it came in via Ralf.
> However I can queue it up locally so it doesn't get forgotten.

Did look good except I recently turned uaccess.h upside down for the
sake of sparse.

  Ralf

Re: [patch][resend] convert a remaining verify_area to access_ok (was: Re: [PATCH 2.6.11-mm1] mips: more convert verify_area to access_ok) (fwd)

[Jesper Juhl]

On Thu, 17 Mar 2005, Ralf Baechle wrote:

> On Wed, Mar 16, 2005 at 10:35:09PM +0100, Jesper Juhl wrote:
> 
> > Around 2.6.11-mm1 Yoichi Yuasa found a user of verify_area that I had 
> > missed when converting everything to access_ok. The patch below still 
> > applies cleanly to 2.6.11-mm4.
> > Please apply (unless of course you already picked it up back then and 
> > have it in a queue somewhere :) .
> 
> Oh gosh, you actually converted the whole IRIX compatibility mess even,
> amazing stomach you have :-) I only noticed that when I just looked at
> Linus' tree - after buring a few hours into cleaning those files myself -
> mine are now almost free of sparse warnings.
> 
I hope I did a descent job and that you didn't waste too much time 
duplicating effort...

> The last instance of verify_area() in the MIPS code is now the definition
> itself.
> 
The plan is to wait for a few months (or a few kernel releases - whichever 
comes first) and then I'll send Andrew patches to remove it completely.
There are still a few related nits left, like the FPU_verify_area function 
arch/i386/math-emu/reg_ld_str.c and the rw_verify_area function in 
fs/read_write.c that I want to get out of the way first (think I'll 
probably end up attempting to rename those s/verify_area/access_ok/ and 
see if people scream).


-- 
Jesper

Re: [patch][resend] convert a remaining verify_area to access_ok (was: Re: [PATCH 2.6.11-mm1] mips: more convert verify_area to access_ok) (fwd)

[Ralf Baechle]

On Fri, Mar 18, 2005 at 01:17:47AM +0100, Jesper Juhl wrote:

> I hope I did a descent job and that you didn't waste too much time 
> duplicating effort...

Didn't look too hard at it since my patch of something like 2,500 lines
should be a superset of yours.

> > The last instance of verify_area() in the MIPS code is now the definition
> > itself.
> > 
> The plan is to wait for a few months (or a few kernel releases - whichever 
> comes first) and then I'll send Andrew patches to remove it completely.
> There are still a few related nits left, like the FPU_verify_area function 
> arch/i386/math-emu/reg_ld_str.c and the rw_verify_area function in 
> fs/read_write.c that I want to get out of the way first (think I'll 
> probably end up attempting to rename those s/verify_area/access_ok/ and 
> see if people scream).

Access_ok was introduced in 2.1.4.  Easy for people to write code that's
portable and so verify_area should die a peaceful death.

  Ralf