might_sleep() in copy_{from,to}_user and friends?

DISCUSSION Manfred Spraul 2 participants 2 messages Nov 1, 2002

Share & tools

Manfred Spraul

author

2002-11-01 13:40 UTC #0 lore raw

Quoted 4 lines · Arnd

I have been looking for more places in 2.5 that can be marked might_sleep() and noticed that all the functions in asm/uaccess.h are not marked although they sleep if the memory they access
has to be paged in.

Good idea.
There is some abuse of __get_user to identify bad pointers:
show_registers in the oops codepath, mm/slab.c in the /proc/slabinfo code.

Could you omit the test from the __ versions?

--
Manfred

Arnd Bergmann

2002-11-01 14:43 UTC #1 lore raw

Quoted 5 lines · Manfred Spraul

Good idea.
There is some abuse of __get_user to identify bad pointers:
show_registers in the oops codepath, mm/slab.c in the /proc/slabinfo code.
Could you omit the test from the __ versions?

That would we the patch below. But shouldn't those abuses of __get_user rather be changed to use something else? The name is just wrong there and the first argument is never used.

We could instead do something like

#define check_pointer(p) ({				\
	typeof(*(p)) __t;				\
	long __gu_err;					\
	__get_user_size(__t, p, sizeof(__t), __gu_err);	\
	__gu_err;					\
})

	Arnd <><

===== arch/i386/lib/usercopy.c 1.8 vs edited =====

arch/i386/lib/usercopy.c +5 -0

@@ -62,6 +62,7 @@
 strncpy_from_user(char *dst, const char *src, long count)
 {
 	long res = -EFAULT;
+	might_sleep();
 	if (access_ok(VERIFY_READ, src, 1))
 		__do_strncpy_from_user(dst, src, count, res);
 	return res;

@@ -96,6 +97,7 @@
 unsigned long
 clear_user(void *to, unsigned long n)
 {
+	might_sleep();
 	if (access_ok(VERIFY_WRITE, to, n))
 		__do_clear_user(to, n);
 	return n;

@@ -119,6 +121,7 @@
 	unsigned long mask = -__addr_ok(s);
 	unsigned long res, tmp;
 
+	might_sleep();
 	__asm__ __volatile__(
 		"	testl %0, %0\n"
 		"	jz 3f\n"

@@ -438,6 +441,7 @@
 unsigned long copy_to_user(void *to, const void *from, unsigned long n)
 {
 	prefetch(from);
+	might_sleep();
 	if (access_ok(VERIFY_WRITE, to, n))
 		n = __copy_to_user(to, from, n);
 	return n;

@@ -446,6 +450,7 @@
 unsigned long copy_from_user(void *to, const void *from, unsigned long n)
 {
 	prefetchw(to);
+	might_sleep();
 	if (access_ok(VERIFY_READ, from, n))
 		n = __copy_from_user(to, from, n);
 	return n;

===== include/asm-i386/uaccess.h 1.12 vs edited =====

include/asm-i386/uaccess.h +3 -1

@@ -123,6 +123,7 @@
 /* Careful: we have to cast the result to the type of the pointer for sign reasons */
 #define get_user(x,ptr)							\
 ({	int __ret_gu,__val_gu;						\
+	might_sleep();							\
 	switch(sizeof (*(ptr))) {					\
 	case 1:  __get_user_x(1,__ret_gu,__val_gu,ptr); break;		\
 	case 2:  __get_user_x(2,__ret_gu,__val_gu,ptr); break;		\

@@ -158,8 +159,9 @@
 
 #define __put_user_check(x,ptr,size)			\
 ({							\
-	long __pu_err = -EFAULT;					\
+	long __pu_err = -EFAULT;			\
 	__typeof__(*(ptr)) *__pu_addr = (ptr);		\
+	might_sleep();					\
 	if (access_ok(VERIFY_WRITE,__pu_addr,size))	\
 		__put_user_size((x),__pu_addr,(size),__pu_err);	\
 	__pu_err;					\