[PATCH] Fix __copy_{to,from}_user_inatomic

[PATCH] Fix __copy_{to,from}_user_inatomic

[Franck Bui-Huu]

From: Franck Bui-Huu <fbuihuu@gmail.com>

These functions are aliases to __copy_{to,from}_user resp but they
are not allowed to sleep. Therefore might_sleep() must not be used
by their implementions.

Signed-off-by: Franck Bui-Huu <fbuihuu@gmail.com>
---
 include/asm-mips/uaccess.h |    6 ++----
 1 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/include/asm-mips/uaccess.h b/include/asm-mips/uaccess.h
index 825fcbd..fd01939 100644
--- a/include/asm-mips/uaccess.h
+++ b/include/asm-mips/uaccess.h
@@ -407,7 +407,7 @@ extern size_t __copy_user(void *__to, const void *__from, size_t __n);
  * @from: Source address, in kernel space.
  * @n:    Number of bytes to copy.
  *
- * Context: User context only.  This function may sleep.
+ * Context: User context only.
  *
  * Copy data from kernel space to user space.  Caller must check
  * the specified block with access_ok() before calling this function.
@@ -421,7 +421,6 @@ extern size_t __copy_user(void *__to, const void *__from, size_t __n);
 	const void *__cu_from;						\
 	long __cu_len;							\
 									\
-	might_sleep();							\
 	__cu_to = (to);							\
 	__cu_from = (from);						\
 	__cu_len = (n);							\
@@ -490,7 +489,7 @@ extern size_t __copy_user(void *__to, const void *__from, size_t __n);
  * @from: Source address, in user space.
  * @n:    Number of bytes to copy.
  *
- * Context: User context only.  This function may sleep.
+ * Context: User context only.
  *
  * Copy data from user space to kernel space.  Caller must check
  * the specified block with access_ok() before calling this function.
@@ -507,7 +506,6 @@ extern size_t __copy_user(void *__to, const void *__from, size_t __n);
 	const void __user *__cu_from;					\
 	long __cu_len;							\
 									\
-	might_sleep();							\
 	__cu_to = (to);							\
 	__cu_from = (from);						\
 	__cu_len = (n);							\
-- 
1.4.4.3.ge6d4

Re: [PATCH] Fix __copy_{to,from}_user_inatomic

[Ralf Baechle]

On Fri, Feb 16, 2007 at 04:32:53PM +0100, Franck Bui-Huu wrote:

> These functions are aliases to __copy_{to,from}_user resp but they
> are not allowed to sleep. Therefore might_sleep() must not be used
> by their implementions.

The _inatomic functions are know to buggy but this doesn't quite fix the
whole issues with them.  On error __copy_from_user_inatomic should not
clear the non-copied part of the destination buffer.  See
01408c4939479ec46c15aa7ef6e2406be50eeeca and
7c12d81134b130ccd4c286b434ca48c4cda71a2f for the rationale.

  Ralf

Re: [PATCH] Fix __copy_{to,from}_user_inatomic

[Franck Bui-Huu]

On 2/16/07, Ralf Baechle <ralf@linux-mips.org> wrote:
> On Fri, Feb 16, 2007 at 04:32:53PM +0100, Franck Bui-Huu wrote:
>
> > These functions are aliases to __copy_{to,from}_user resp but they
> > are not allowed to sleep. Therefore might_sleep() must not be used
> > by their implementions.
>
> The _inatomic functions are know to buggy but this doesn't quite fix the
> whole issues with them.

ok so the patch's should have been: "Fix console warnings triggered by
__copy_{to,from}_user_inatomic()"

> On error __copy_from_user_inatomic should not
> clear the non-copied part of the destination buffer.  See
> 01408c4939479ec46c15aa7ef6e2406be50eeeca and
> 7c12d81134b130ccd4c286b434ca48c4cda71a2f for the rationale.

I'll try to take a look at it.

Thanks
-- 
               Franck

Re: [PATCH] Fix __copy_{to,from}_user_inatomic

[Atsushi Nemoto]

On Fri, 16 Feb 2007 16:32:53 +0100, Franck Bui-Huu <vagabon.xyz@gmail.com> wrote:
> These functions are aliases to __copy_{to,from}_user resp but they
> are not allowed to sleep. Therefore might_sleep() must not be used
> by their implementions.

This changes CONFIG_PREEMPT_VOLUNTARY behavior.
In this case might_sleep() is not just an annotation.

Well, so currently CONFIG_PREEMPT_VOLUNTARY seems broken.  Maybe we
need separete functions anyway.
---
Atsushi Nemoto

Re: [PATCH] Fix __copy_{to,from}_user_inatomic

[Ralf Baechle]

On Sat, Feb 17, 2007 at 01:12:20AM +0900, Atsushi Nemoto wrote:
> Date:	Sat, 17 Feb 2007 01:12:20 +0900 (JST)
> To:	vagabon.xyz@gmail.com
> Cc:	ralf@linux-mips.org, linux-mips@linux-mips.org
> Subject: Re: [PATCH] Fix __copy_{to,from}_user_inatomic
> From:	Atsushi Nemoto <anemo@mba.ocn.ne.jp>
> Content-Type: Text/Plain; charset=us-ascii
> 
> On Fri, 16 Feb 2007 16:32:53 +0100, Franck Bui-Huu <vagabon.xyz@gmail.com> wrote:
> > These functions are aliases to __copy_{to,from}_user resp but they
> > are not allowed to sleep. Therefore might_sleep() must not be used
> > by their implementions.
> 
> This changes CONFIG_PREEMPT_VOLUNTARY behavior.
> In this case might_sleep() is not just an annotation.
> 
> Well, so currently CONFIG_PREEMPT_VOLUNTARY seems broken.  Maybe we
> need separete functions anyway.

The issue needed to be tackled so while suboptimal I've added a chainsawed
version of memcpy to provide __copy_from_user_inatomic.  Not elegant but
hopefully that will be cleaned soon.  memcpy could use a general ovehaul
anyway.

  Ralf