fix verify_write for 80386 support

RFC PATCH In review Manfred Spraul 2 participants 3 messages Apr 21, 2003 – Apr 22, 2003

Share & tools

Manfred Spraul

author

2003-04-21 20:16 UTC #0 lore raw

RFC PATCH 1/1 fix verify_write for 80386 support

Real 80386 cpus have a special feature: they ignore the write-protected bit in the page tables if the kernel runs at ring 0. This feature breaks COW, the software must manually check the write protected bit before writing.

Right now the check happens only in access_ok(VERIFY_WRITE).
This is broken, because:
- some codepaths call access_ok(VERIFY_READ), and then write.
- kswapd might change the page permissions between access_ok and the actual write access.

Manfred Spraul

author

2003-04-21 20:24 UTC #1 lore raw

[ 2nd part of the mail, after hitting the send button in the middle of the sentence]

The solution is to perform the check for write protection in the actual access functions, not during access_ok.

The attached patch does that:
- remove the check from access_ok
- redirect all user space write access into __copy_to_user_ll
- within __copy_to_user_ll: use get_user_pages, and write directly to the obtained kernel address.

This fixes all swapout & data corruption bugs, and even removes 26 lines from the kernel.

What do you think? The alternative would be to drop support for real 80386 cpus.

Not tested on a real 80386, I've just replace wp_works_ok with !wp_works_ok.

--
Manfred

 arch/i386/lib/usercopy.c   |   47 ++++++++++++++++++++++++++
 arch/i386/mm/fault.c       |   81 ---------------------------------------------
 include/asm-i386/uaccess.h |   30 +++++++---------
 3 files changed, 61 insertions(+), 97 deletions(-)
// $Header$
// Kernel Version:
//  VERSION = 2
//  PATCHLEVEL = 5
//  SUBLEVEL = 68
//  EXTRAVERSION =

include/asm-i386/uaccess.h +14 ~~-16~~

@@ -62,8 +62,6 @@
 		:"1" (addr),"g" ((int)(size)),"g" (current_thread_info()->addr_limit.seg)); \
 	flag; })
 
-#ifdef CONFIG_X86_WP_WORKS_OK
-
 /**
  * access_ok: - Checks if a user space pointer is valid
  * @type: Type of access: %VERIFY_READ or %VERIFY_WRITE.  Note that

@@ -85,14 +83,6 @@
  */
 #define access_ok(type,addr,size) (__range_ok(addr,size) == 0)
 
-#else
-
-#define access_ok(type,addr,size) ( (__range_ok(addr,size) == 0) && \
-			 ((type) == VERIFY_READ || boot_cpu_data.wp_works_ok || \
-			  __verify_write((void *)(addr),(size))))
-
-#endif
-
 /**
  * verify_area: - Obsolete, use access_ok()
  * @type: Type of access: %VERIFY_READ or %VERIFY_WRITE

@@ -191,14 +181,8 @@
 	__ret_gu;							\
 })
 
-extern void __put_user_1(void);
-extern void __put_user_2(void);
-extern void __put_user_4(void);
-extern void __put_user_8(void);
-
 extern void __put_user_bad(void);
 
-
 /**
  * put_user: - Write a simple value into user space.
  * @x:   Value to copy to user space.

@@ -299,6 +283,8 @@
 		: "=r"(err)					\
 		: "A" (x), "r" (addr), "i"(-EFAULT), "0"(err))
 
+#ifdef CONFIG_X86_WP_WORKS_OK
+
 #define __put_user_size(x,ptr,size,retval,errret)			\
 do {									\
 	retval = 0;							\

@@ -311,6 +297,18 @@
 	}								\
 } while (0)
 
+#else
+
+#define __put_user_size(x,ptr,size,retval,errret)			\
+do {									\
+	__typeof__(*(ptr)) __pus_tmp = x;				\
+	retval = 0;							\
+									\
+	if(unlikely(__copy_to_user_ll(ptr, &__pus_tmp, size) != 0))	\
+		retval = errret;					\
+} while (0)
+
+#endif
 struct __large_struct { unsigned long buf[100]; };
 #define __m(x) (*(struct __large_struct *)(x))

arch/i386/mm/fault.c +0 ~~-81~~

87 lines not on this page · raw message

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

59 lines not on this page · raw message

Brian Gerst

2003-04-22 02:02 UTC #2 lore raw

Quoted 10 lines · Manfred Spraul

[ 2nd part of the mail, after hitting the send button in the middle of the sentence]
The solution is to perform the check for write protection in the actual access functions, not during access_ok.
The attached patch does that:
- remove the check from access_ok
- redirect all user space write access into __copy_to_user_ll
- within __copy_to_user_ll: use get_user_pages, and write directly to the obtained kernel address.

Quoted 9 lines

This fixes all swapout & data corruption bugs, and even removes 26 lines from the kernel.
What do you think? The alternative would be to drop support for real 80386 cpus.
Not tested on a real 80386, I've just replace wp_works_ok with !wp_works_ok.
Signature · 1 line
   Manfred

Close, but there is still a race here. You need to hold mmap_sem over the memcpy. Another thread will expect the memory to not be written to after write-protecting it.

--
Brian Gerst