All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC -tip 0/4] x86: improve uaccess in signal
@ 2008-12-23  5:20 Hiroshi Shimamoto
  2008-12-23  5:22 ` [RFC -tip 1/4] x86: uaccess: rename __put_user_u64() to __put_user_asm_u64() Hiroshi Shimamoto
                   ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: Hiroshi Shimamoto @ 2008-12-23  5:20 UTC (permalink / raw)
  To: Ingo Molnar, Thomas Gleixner, H. Peter Anvin; +Cc: linux-kernel

This patch series is experimental.

This is another proposal to improve uaccess in signal.
This patch series will improve signal handling performance. The normal path of
signal handling should be same as this;
http://lkml.org/lkml/2008/9/25/332

I think there is a lot of fixup code in signal generated by __{get|put}_user().
In most case kernel only needs to know whether there is error, and error value
is not important. And when an exception has occurred, kernel doesn't need to
continue following __{get|put}_user() in that function.

I think in fixup code to use direct jump to the tail of function and returns
error, will save fixup code size. This fixup code can be used __{get|put}_user()
calls in same function.

The concept code is;
int func()
{
	int err = 0;
.section .fixup
error_out:
	err = -EFAULT;
	goto out;
.previous
	__{get|put}_user(); /* fixup: jump to error_out */
	__{get|put}_user();
	__{get|put}_user();
	:
out:
	return err;
}

and the results are;
$ size *signal*.o.*
   text	   data	    bss	    dec	    hex	filename
   4646	      0	      0	   4646	   1226	ia32_signal.o.new
   6006	      0	      0	   6006	   1776	ia32_signal.o.old
   3557	      0	      0	   3557	    de5	signal.o.new
   4540	      0	      0	   4540	   11bc	signal.o.old
   3840	      0	      0	   3840	    f00	signal32.o.new
   4876	      0	      0	   4876	   130c	signal32.o.old

However, this code is tricky, I can't say compiler guarantees to keep the
order of this framework code and this code relies on the order, fixup section
is before the all __{get|put}_user() codes and the out label is just before
the return.

Comments are welcome.

Thanks,
Hiroshi

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [RFC -tip 1/4] x86: uaccess: rename __put_user_u64() to __put_user_asm_u64()
  2008-12-23  5:20 [RFC -tip 0/4] x86: improve uaccess in signal Hiroshi Shimamoto
@ 2008-12-23  5:22 ` Hiroshi Shimamoto
  2008-12-23  5:22 ` [RFC -tip 2/4] x86: uaccess: introduce __{get|put}_user exception handling framework Hiroshi Shimamoto
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 9+ messages in thread
From: Hiroshi Shimamoto @ 2008-12-23  5:22 UTC (permalink / raw)
  To: Ingo Molnar, Thomas Gleixner, H. Peter Anvin; +Cc: linux-kernel

From: Hiroshi Shimamoto <h-shimamoto@ct.jp.nec.com>

Impact: cleanup

Rename __put_user_u64() to __put_user_asm_u64() like __get_user_asm_u64().

Signed-off-by: Hiroshi Shimamoto <h-shimamoto@ct.jp.nec.com>
---
 arch/x86/include/asm/uaccess.h |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/x86/include/asm/uaccess.h b/arch/x86/include/asm/uaccess.h
index 4340055..1a38180 100644
--- a/arch/x86/include/asm/uaccess.h
+++ b/arch/x86/include/asm/uaccess.h
@@ -186,7 +186,7 @@ extern int __get_user_bad(void);
 
 
 #ifdef CONFIG_X86_32
-#define __put_user_u64(x, addr, err)					\
+#define __put_user_asm_u64(x, addr, err)				\
 	asm volatile("1:	movl %%eax,0(%2)\n"			\
 		     "2:	movl %%edx,4(%2)\n"			\
 		     "3:\n"						\
@@ -203,7 +203,7 @@ extern int __get_user_bad(void);
 	asm volatile("call __put_user_8" : "=a" (__ret_pu)	\
 		     : "A" ((typeof(*(ptr)))(x)), "c" (ptr) : "ebx")
 #else
-#define __put_user_u64(x, ptr, retval) \
+#define __put_user_asm_u64(x, ptr, retval) \
 	__put_user_asm(x, ptr, retval, "q", "", "Zr", -EFAULT)
 #define __put_user_x8(x, ptr, __ret_pu) __put_user_x(8, x, ptr, __ret_pu)
 #endif
@@ -279,7 +279,7 @@ do {									\
 		__put_user_asm(x, ptr, retval, "l", "k",  "ir", errret);\
 		break;							\
 	case 8:								\
-		__put_user_u64((__typeof__(*ptr))(x), ptr, retval);	\
+		__put_user_asm_u64((__typeof__(*ptr))(x), ptr, retval);	\
 		break;							\
 	default:							\
 		__put_user_bad();					\
-- 
1.6.0.4


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [RFC -tip 2/4] x86: uaccess: introduce __{get|put}_user exception handling framework
  2008-12-23  5:20 [RFC -tip 0/4] x86: improve uaccess in signal Hiroshi Shimamoto
  2008-12-23  5:22 ` [RFC -tip 1/4] x86: uaccess: rename __put_user_u64() to __put_user_asm_u64() Hiroshi Shimamoto
@ 2008-12-23  5:22 ` Hiroshi Shimamoto
  2008-12-23  5:38   ` Brian Gerst
  2008-12-23 14:30   ` Ingo Molnar
  2008-12-23  5:23 ` [RFC -tip 3/4] x86: signal: use " Hiroshi Shimamoto
  2008-12-23  5:24 ` [RFC -tip 4/4] x86: ia32_signal: " Hiroshi Shimamoto
  3 siblings, 2 replies; 9+ messages in thread
From: Hiroshi Shimamoto @ 2008-12-23  5:22 UTC (permalink / raw)
  To: Ingo Molnar, Thomas Gleixner, H. Peter Anvin; +Cc: linux-kernel

From: Hiroshi Shimamoto <h-shimamoto@ct.jp.nec.com>

Impact: introduce new framework

Introduce exception handling framework.
__{get|put}_user_ex_try() begins exception block and
__{get|put}_user_ex_catch() ends block and if an exception occurred in this
block using __{get|put}_user_ex, direct jump to __{get|put}_user_ex_catch()
and err is set to specified value.

int func()
{
	int err = 0;

	__get_user_ex_try(&err, -EFAULT);

	__get_user_ex(...);
	__get_user_ex(...);
	:

	__get_user_ex_catch();
	return err;
}

Signed-off-by: Hiroshi Shimamoto <h-shimamoto@ct.jp.nec.com>
---
 arch/x86/include/asm/uaccess.h |  110 ++++++++++++++++++++++++++++++++++++++++
 1 files changed, 110 insertions(+), 0 deletions(-)

diff --git a/arch/x86/include/asm/uaccess.h b/arch/x86/include/asm/uaccess.h
index 1a38180..cf293fe 100644
--- a/arch/x86/include/asm/uaccess.h
+++ b/arch/x86/include/asm/uaccess.h
@@ -199,12 +199,21 @@ extern int __get_user_bad(void);
 		     : "=r" (err)					\
 		     : "A" (x), "r" (addr), "i" (-EFAULT), "0" (err))
 
+#define __put_user_asm_ex_u64(x, addr, label)				\
+	asm volatile("1:	movl %%eax,0(%1)\n"			\
+		     "2:	movl %%edx,4(%1)\n"			\
+		     _ASM_EXTABLE(1b, label)				\
+		     _ASM_EXTABLE(2b, label)				\
+		     : : "A" (x), "r" (addr))
+
 #define __put_user_x8(x, ptr, __ret_pu)				\
 	asm volatile("call __put_user_8" : "=a" (__ret_pu)	\
 		     : "A" ((typeof(*(ptr)))(x)), "c" (ptr) : "ebx")
 #else
 #define __put_user_asm_u64(x, ptr, retval) \
 	__put_user_asm(x, ptr, retval, "q", "", "Zr", -EFAULT)
+#define __put_user_asm_ex_u64(x, addr, label)	\
+	__put_user_asm_ex(x, addr, "q", "", "Zr", label)
 #define __put_user_x8(x, ptr, __ret_pu) __put_user_x(8, x, ptr, __ret_pu)
 #endif
 
@@ -286,6 +295,27 @@ do {									\
 	}								\
 } while (0)
 
+#define __put_user_size_ex(x, ptr, size, label)			\
+do {									\
+	__chk_user_ptr(ptr);						\
+	switch (size) {							\
+	case 1:								\
+		__put_user_asm_ex(x, ptr, "b", "b", "iq", label);	\
+		break;							\
+	case 2:								\
+		__put_user_asm_ex(x, ptr, "w", "w", "ir", label);	\
+		break;							\
+	case 4:								\
+		__put_user_asm_ex(x, ptr, "l", "k",  "ir", label);	\
+		break;							\
+	case 8:								\
+		__put_user_asm_ex_u64((__typeof__(*ptr))(x), ptr, label);\
+		break;							\
+	default:							\
+		__put_user_bad();					\
+	}								\
+} while (0)
+
 #else
 
 #define __put_user_size(x, ptr, size, retval, errret)			\
@@ -311,9 +341,12 @@ do {									\
 
 #ifdef CONFIG_X86_32
 #define __get_user_asm_u64(x, ptr, retval, errret)	(x) = __get_user_bad()
+#define __get_user_asm_ex_u64(x, ptr, label)		(x) = __get_user_bad()
 #else
 #define __get_user_asm_u64(x, ptr, retval, errret) \
 	 __get_user_asm(x, ptr, retval, "q", "", "=r", errret)
+#define __get_user_asm_ex_u64(x, ptr, label) \
+	 __get_user_asm_ex(x, ptr, "q", "", "=r", label)
 #endif
 
 #define __get_user_size(x, ptr, size, retval, errret)			\
@@ -350,6 +383,36 @@ do {									\
 		     : "=r" (err), ltype(x)				\
 		     : "m" (__m(addr)), "i" (errret), "0" (err))
 
+#define __get_user_size_ex(x, ptr, size, label)				\
+do {									\
+	__chk_user_ptr(ptr);						\
+	switch (size) {							\
+	case 1:								\
+		__get_user_asm_ex(x, ptr, "b", "b", "=q", label);	\
+		break;							\
+	case 2:								\
+		__get_user_asm_ex(x, ptr, "w", "w", "=r", label);	\
+		break;							\
+	case 4:								\
+		__get_user_asm_ex(x, ptr, "l", "k", "=r", label);	\
+		break;							\
+	case 8:								\
+		__get_user_asm_ex_u64(x, ptr, label);			\
+		break;							\
+	default:							\
+		(x) = __get_user_bad();					\
+	}								\
+} while (0)
+
+#define __get_user_asm_ex(x, addr, itype, rtype, ltype, label)		\
+	asm volatile("1:	mov"itype" %1,%"rtype"0\n"		\
+		     ".section .fixup,\"ax\"\n"				\
+		     "2:	xor"itype" %"rtype"0,%"rtype"0\n"	\
+		     "	jmp " #label "\n"				\
+		     ".previous\n"					\
+		     _ASM_EXTABLE(1b, 2b)				\
+		     : ltype(x) : "m" (__m(addr)))
+
 #define __put_user_nocheck(x, ptr, size)			\
 ({								\
 	int __pu_err;						\
@@ -366,6 +429,16 @@ do {									\
 	__gu_err;							\
 })
 
+#define __put_user_ex_label(x, ptr, size, label) do {			\
+	__put_user_size_ex((x), (ptr), (size), label);			\
+} while (0)
+
+#define __get_user_ex_label(x, ptr, size, label) do {			\
+	unsigned long __gue_val;					\
+	__get_user_size_ex((__gue_val), (ptr), (size), label);		\
+	(x) = (__force __typeof__(*(ptr)))__gue_val;			\
+} while (0)
+
 /* FIXME: this hack is definitely wrong -AK */
 struct __large_struct { unsigned long buf[100]; };
 #define __m(x) (*(struct __large_struct __user *)(x))
@@ -385,6 +458,23 @@ struct __large_struct { unsigned long buf[100]; };
 		     _ASM_EXTABLE(1b, 3b)				\
 		     : "=r"(err)					\
 		     : ltype(x), "m" (__m(addr)), "i" (errret), "0" (err))
+
+#define __put_user_asm_ex(x, addr, itype, rtype, ltype, label)		\
+	asm volatile("1:	mov"itype" %"rtype"0,%1\n"		\
+		     _ASM_EXTABLE(1b, label)				\
+		     : : ltype(x), "m" (__m(addr)))
+
+#define __ex_try_label(err, errval, label, out_label) do {	\
+	asm volatile(".section .fixup,\"ax\"\n"			\
+		     #label ":	mov %1,%0\n"			\
+		     "	jmp " #out_label "\n"			\
+		     ".previous\n"				\
+		     : "=r" (err) : "i" (errval), "0" (err))
+
+#define __ex_catch_label(label)		\
+	asm volatile(#label ":\n");	\
+} while (0)
+
 /**
  * __get_user: - Get a simple variable from user space, with less checking.
  * @x:   Variable to store result.
@@ -408,6 +498,16 @@ struct __large_struct { unsigned long buf[100]; };
 
 #define __get_user(x, ptr)						\
 	__get_user_nocheck((x), (ptr), sizeof(*(ptr)))
+
+#define __get_user_ex(x, ptr)						\
+	__get_user_ex_label((x), (ptr), sizeof(*(ptr)), 880b)
+
+#define __get_user_ex_try(perr, errval)	\
+	__ex_try_label((*(perr)), (errval), 880, 881f)
+
+#define __get_user_ex_catch()	\
+	__ex_catch_label(881)
+
 /**
  * __put_user: - Write a simple value into user space, with less checking.
  * @x:   Value to copy to user space.
@@ -431,6 +531,16 @@ struct __large_struct { unsigned long buf[100]; };
 #define __put_user(x, ptr)						\
 	__put_user_nocheck((__typeof__(*(ptr)))(x), (ptr), sizeof(*(ptr)))
 
+#define __put_user_ex(x, ptr)						\
+	__put_user_ex_label((__typeof__(*(ptr)))(x), (ptr),		\
+			   sizeof(*(ptr)), 882b)
+
+#define __put_user_ex_try(perr, errval) \
+	__ex_try_label((*(perr)), (errval), 882, 883f)
+
+#define __put_user_ex_catch()	\
+	__ex_catch_label(883)
+
 #define __get_user_unaligned __get_user
 #define __put_user_unaligned __put_user
 
-- 
1.6.0.4


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [RFC -tip 3/4] x86: signal: use __{get|put}_user exception handling framework
  2008-12-23  5:20 [RFC -tip 0/4] x86: improve uaccess in signal Hiroshi Shimamoto
  2008-12-23  5:22 ` [RFC -tip 1/4] x86: uaccess: rename __put_user_u64() to __put_user_asm_u64() Hiroshi Shimamoto
  2008-12-23  5:22 ` [RFC -tip 2/4] x86: uaccess: introduce __{get|put}_user exception handling framework Hiroshi Shimamoto
@ 2008-12-23  5:23 ` Hiroshi Shimamoto
  2008-12-23  5:24 ` [RFC -tip 4/4] x86: ia32_signal: " Hiroshi Shimamoto
  3 siblings, 0 replies; 9+ messages in thread
From: Hiroshi Shimamoto @ 2008-12-23  5:23 UTC (permalink / raw)
  To: Ingo Molnar, Thomas Gleixner, H. Peter Anvin; +Cc: linux-kernel

From: Hiroshi Shimamoto <h-shimamoto@ct.jp.nec.com>

Impact: cleanup

Use __{get|put}_user exception handling framework.

Signed-off-by: Hiroshi Shimamoto <h-shimamoto@ct.jp.nec.com>
---
 arch/x86/kernel/signal.c |  186 ++++++++++++++++++++++++---------------------
 1 files changed, 99 insertions(+), 87 deletions(-)

diff --git a/arch/x86/kernel/signal.c b/arch/x86/kernel/signal.c
index 4fa5243..fcaa318 100644
--- a/arch/x86/kernel/signal.c
+++ b/arch/x86/kernel/signal.c
@@ -51,24 +51,24 @@
 #endif
 
 #define COPY(x)			{		\
-	err |= __get_user(regs->x, &sc->x);	\
+	__get_user_ex(regs->x, &sc->x);		\
 }
 
 #define COPY_SEG(seg)		{			\
 		unsigned short tmp;			\
-		err |= __get_user(tmp, &sc->seg);	\
+		__get_user_ex(tmp, &sc->seg);		\
 		regs->seg = tmp;			\
 }
 
 #define COPY_SEG_CPL3(seg)	{			\
 		unsigned short tmp;			\
-		err |= __get_user(tmp, &sc->seg);	\
+		__get_user_ex(tmp, &sc->seg);		\
 		regs->seg = tmp | 3;			\
 }
 
 #define GET_SEG(seg)		{			\
 		unsigned short tmp;			\
-		err |= __get_user(tmp, &sc->seg);	\
+		__get_user_ex(tmp, &sc->seg);		\
 		loadsegment(seg, tmp);			\
 }
 
@@ -83,6 +83,8 @@ restore_sigcontext(struct pt_regs *regs, struct sigcontext __user *sc,
 	/* Always make any pending restarted system calls return -EINTR */
 	current_thread_info()->restart_block.fn = do_no_restart_syscall;
 
+	__get_user_ex_try(&err, -EFAULT);
+
 #ifdef CONFIG_X86_32
 	GET_SEG(gs);
 	COPY_SEG(fs);
@@ -114,14 +116,16 @@ restore_sigcontext(struct pt_regs *regs, struct sigcontext __user *sc,
 	COPY_SEG_CPL3(cs);
 #endif /* CONFIG_X86_32 */
 
-	err |= __get_user(tmpflags, &sc->flags);
+	__get_user_ex(tmpflags, &sc->flags);
 	regs->flags = (regs->flags & ~FIX_EFLAGS) | (tmpflags & FIX_EFLAGS);
 	regs->orig_ax = -1;		/* disable syscall checks */
 
-	err |= __get_user(buf, &sc->fpstate);
+	__get_user_ex(buf, &sc->fpstate);
 	err |= restore_i387_xstate(buf);
 
-	err |= __get_user(*pax, &sc->ax);
+	__get_user_ex(*pax, &sc->ax);
+
+	__get_user_ex_catch();
 	return err;
 }
 
@@ -131,57 +135,61 @@ setup_sigcontext(struct sigcontext __user *sc, void __user *fpstate,
 {
 	int err = 0;
 
+	__put_user_ex_try(&err, -EFAULT);
+
 #ifdef CONFIG_X86_32
 	{
 		unsigned int tmp;
 
 		savesegment(gs, tmp);
-		err |= __put_user(tmp, (unsigned int __user *)&sc->gs);
+		__put_user_ex(tmp, (unsigned int __user *)&sc->gs);
 	}
-	err |= __put_user(regs->fs, (unsigned int __user *)&sc->fs);
-	err |= __put_user(regs->es, (unsigned int __user *)&sc->es);
-	err |= __put_user(regs->ds, (unsigned int __user *)&sc->ds);
+	__put_user_ex(regs->fs, (unsigned int __user *)&sc->fs);
+	__put_user_ex(regs->es, (unsigned int __user *)&sc->es);
+	__put_user_ex(regs->ds, (unsigned int __user *)&sc->ds);
 #endif /* CONFIG_X86_32 */
 
-	err |= __put_user(regs->di, &sc->di);
-	err |= __put_user(regs->si, &sc->si);
-	err |= __put_user(regs->bp, &sc->bp);
-	err |= __put_user(regs->sp, &sc->sp);
-	err |= __put_user(regs->bx, &sc->bx);
-	err |= __put_user(regs->dx, &sc->dx);
-	err |= __put_user(regs->cx, &sc->cx);
-	err |= __put_user(regs->ax, &sc->ax);
+	__put_user_ex(regs->di, &sc->di);
+	__put_user_ex(regs->si, &sc->si);
+	__put_user_ex(regs->bp, &sc->bp);
+	__put_user_ex(regs->sp, &sc->sp);
+	__put_user_ex(regs->bx, &sc->bx);
+	__put_user_ex(regs->dx, &sc->dx);
+	__put_user_ex(regs->cx, &sc->cx);
+	__put_user_ex(regs->ax, &sc->ax);
 #ifdef CONFIG_X86_64
-	err |= __put_user(regs->r8, &sc->r8);
-	err |= __put_user(regs->r9, &sc->r9);
-	err |= __put_user(regs->r10, &sc->r10);
-	err |= __put_user(regs->r11, &sc->r11);
-	err |= __put_user(regs->r12, &sc->r12);
-	err |= __put_user(regs->r13, &sc->r13);
-	err |= __put_user(regs->r14, &sc->r14);
-	err |= __put_user(regs->r15, &sc->r15);
+	__put_user_ex(regs->r8, &sc->r8);
+	__put_user_ex(regs->r9, &sc->r9);
+	__put_user_ex(regs->r10, &sc->r10);
+	__put_user_ex(regs->r11, &sc->r11);
+	__put_user_ex(regs->r12, &sc->r12);
+	__put_user_ex(regs->r13, &sc->r13);
+	__put_user_ex(regs->r14, &sc->r14);
+	__put_user_ex(regs->r15, &sc->r15);
 #endif /* CONFIG_X86_64 */
 
-	err |= __put_user(current->thread.trap_no, &sc->trapno);
-	err |= __put_user(current->thread.error_code, &sc->err);
-	err |= __put_user(regs->ip, &sc->ip);
+	__put_user_ex(current->thread.trap_no, &sc->trapno);
+	__put_user_ex(current->thread.error_code, &sc->err);
+	__put_user_ex(regs->ip, &sc->ip);
 #ifdef CONFIG_X86_32
-	err |= __put_user(regs->cs, (unsigned int __user *)&sc->cs);
-	err |= __put_user(regs->flags, &sc->flags);
-	err |= __put_user(regs->sp, &sc->sp_at_signal);
-	err |= __put_user(regs->ss, (unsigned int __user *)&sc->ss);
+	__put_user_ex(regs->cs, (unsigned int __user *)&sc->cs);
+	__put_user_ex(regs->flags, &sc->flags);
+	__put_user_ex(regs->sp, &sc->sp_at_signal);
+	__put_user_ex(regs->ss, (unsigned int __user *)&sc->ss);
 #else /* !CONFIG_X86_32 */
-	err |= __put_user(regs->flags, &sc->flags);
-	err |= __put_user(regs->cs, &sc->cs);
-	err |= __put_user(0, &sc->gs);
-	err |= __put_user(0, &sc->fs);
+	__put_user_ex(regs->flags, &sc->flags);
+	__put_user_ex(regs->cs, &sc->cs);
+	__put_user_ex(0, &sc->gs);
+	__put_user_ex(0, &sc->fs);
 #endif /* CONFIG_X86_32 */
 
-	err |= __put_user(fpstate, &sc->fpstate);
+	__put_user_ex(fpstate, &sc->fpstate);
 
 	/* non-iBCS2 extensions.. */
-	err |= __put_user(mask, &sc->oldmask);
-	err |= __put_user(current->thread.cr2, &sc->cr2);
+	__put_user_ex(mask, &sc->oldmask);
+	__put_user_ex(current->thread.cr2, &sc->cr2);
+
+	__put_user_ex_catch();
 
 	return err;
 }
@@ -269,13 +277,14 @@ __setup_frame(int sig, struct k_sigaction *ka, sigset_t *set,
 	int err = 0;
 	void __user *fpstate = NULL;
 
+	__put_user_ex_try(&err, -EFAULT);
+
 	frame = get_sigframe(ka, regs, sizeof(*frame), &fpstate);
 
 	if (!access_ok(VERIFY_WRITE, frame, sizeof(*frame)))
 		return -EFAULT;
 
-	if (__put_user(sig, &frame->sig))
-		return -EFAULT;
+	__put_user_ex(sig, &frame->sig);
 
 	if (setup_sigcontext(&frame->sc, fpstate, regs, set->sig[0]))
 		return -EFAULT;
@@ -294,7 +303,7 @@ __setup_frame(int sig, struct k_sigaction *ka, sigset_t *set,
 		restorer = ka->sa.sa_restorer;
 
 	/* Set up to return from userspace.  */
-	err |= __put_user(restorer, &frame->pretcode);
+	__put_user_ex(restorer, &frame->pretcode);
 
 	/*
 	 * This is popl %eax ; movl $__NR_sigreturn, %eax ; int $0x80
@@ -303,10 +312,7 @@ __setup_frame(int sig, struct k_sigaction *ka, sigset_t *set,
 	 * reasons and because gdb uses it as a signature to notice
 	 * signal handler stack frames.
 	 */
-	err |= __put_user(*((u64 *)&retcode), (u64 *)frame->retcode);
-
-	if (err)
-		return -EFAULT;
+	__put_user_ex(*((u64 *)&retcode), (u64 *)frame->retcode);
 
 	/* Set up registers for signal handler */
 	regs->sp = (unsigned long)frame;
@@ -320,7 +326,9 @@ __setup_frame(int sig, struct k_sigaction *ka, sigset_t *set,
 	regs->ss = __USER_DS;
 	regs->cs = __USER_CS;
 
-	return 0;
+	__put_user_ex_catch();
+
+	return err;
 }
 
 static int __setup_rt_frame(int sig, struct k_sigaction *ka, siginfo_t *info,
@@ -331,28 +339,30 @@ static int __setup_rt_frame(int sig, struct k_sigaction *ka, siginfo_t *info,
 	int err = 0;
 	void __user *fpstate = NULL;
 
+	__put_user_ex_try(&err, -EFAULT);
+
 	frame = get_sigframe(ka, regs, sizeof(*frame), &fpstate);
 
 	if (!access_ok(VERIFY_WRITE, frame, sizeof(*frame)))
 		return -EFAULT;
 
-	err |= __put_user(sig, &frame->sig);
-	err |= __put_user(&frame->info, &frame->pinfo);
-	err |= __put_user(&frame->uc, &frame->puc);
+	__put_user_ex(sig, &frame->sig);
+	__put_user_ex(&frame->info, &frame->pinfo);
+	__put_user_ex(&frame->uc, &frame->puc);
 	err |= copy_siginfo_to_user(&frame->info, info);
 	if (err)
 		return -EFAULT;
 
 	/* Create the ucontext.  */
 	if (cpu_has_xsave)
-		err |= __put_user(UC_FP_XSTATE, &frame->uc.uc_flags);
+		__put_user_ex(UC_FP_XSTATE, &frame->uc.uc_flags);
 	else
-		err |= __put_user(0, &frame->uc.uc_flags);
-	err |= __put_user(0, &frame->uc.uc_link);
-	err |= __put_user(current->sas_ss_sp, &frame->uc.uc_stack.ss_sp);
-	err |= __put_user(sas_ss_flags(regs->sp),
+		__put_user_ex(0, &frame->uc.uc_flags);
+	__put_user_ex(0, &frame->uc.uc_link);
+	__put_user_ex(current->sas_ss_sp, &frame->uc.uc_stack.ss_sp);
+	__put_user_ex(sas_ss_flags(regs->sp),
 			  &frame->uc.uc_stack.ss_flags);
-	err |= __put_user(current->sas_ss_size, &frame->uc.uc_stack.ss_size);
+	__put_user_ex(current->sas_ss_size, &frame->uc.uc_stack.ss_size);
 	err |= setup_sigcontext(&frame->uc.uc_mcontext, fpstate,
 				regs, set->sig[0]);
 	err |= __copy_to_user(&frame->uc.uc_sigmask, set, sizeof(*set));
@@ -363,7 +373,7 @@ static int __setup_rt_frame(int sig, struct k_sigaction *ka, siginfo_t *info,
 	restorer = VDSO32_SYMBOL(current->mm->context.vdso, rt_sigreturn);
 	if (ka->sa.sa_flags & SA_RESTORER)
 		restorer = ka->sa.sa_restorer;
-	err |= __put_user(restorer, &frame->pretcode);
+	__put_user_ex(restorer, &frame->pretcode);
 
 	/*
 	 * This is movl $__NR_rt_sigreturn, %ax ; int $0x80
@@ -372,10 +382,7 @@ static int __setup_rt_frame(int sig, struct k_sigaction *ka, siginfo_t *info,
 	 * reasons and because gdb uses it as a signature to notice
 	 * signal handler stack frames.
 	 */
-	err |= __put_user(*((u64 *)&rt_retcode), (u64 *)frame->retcode);
-
-	if (err)
-		return -EFAULT;
+	__put_user_ex(*((u64 *)&rt_retcode), (u64 *)frame->retcode);
 
 	/* Set up registers for signal handler */
 	regs->sp = (unsigned long)frame;
@@ -389,7 +396,9 @@ static int __setup_rt_frame(int sig, struct k_sigaction *ka, siginfo_t *info,
 	regs->ss = __USER_DS;
 	regs->cs = __USER_CS;
 
-	return 0;
+	__put_user_ex_catch();
+
+	return err;
 }
 #else /* !CONFIG_X86_32 */
 /*
@@ -418,6 +427,8 @@ static int __setup_rt_frame(int sig, struct k_sigaction *ka, siginfo_t *info,
 	int err = 0;
 	struct task_struct *me = current;
 
+	__put_user_ex_try(&err, -EFAULT);
+
 	if (used_math()) {
 		fp = get_stack(ka, regs->sp, sig_xstate_size);
 		frame = (void __user *)round_down(
@@ -438,14 +449,14 @@ static int __setup_rt_frame(int sig, struct k_sigaction *ka, siginfo_t *info,
 
 	/* Create the ucontext.  */
 	if (cpu_has_xsave)
-		err |= __put_user(UC_FP_XSTATE, &frame->uc.uc_flags);
+		__put_user_ex(UC_FP_XSTATE, &frame->uc.uc_flags);
 	else
-		err |= __put_user(0, &frame->uc.uc_flags);
-	err |= __put_user(0, &frame->uc.uc_link);
-	err |= __put_user(me->sas_ss_sp, &frame->uc.uc_stack.ss_sp);
-	err |= __put_user(sas_ss_flags(regs->sp),
+		__put_user_ex(0, &frame->uc.uc_flags);
+	__put_user_ex(0, &frame->uc.uc_link);
+	__put_user_ex(me->sas_ss_sp, &frame->uc.uc_stack.ss_sp);
+	__put_user_ex(sas_ss_flags(regs->sp),
 			  &frame->uc.uc_stack.ss_flags);
-	err |= __put_user(me->sas_ss_size, &frame->uc.uc_stack.ss_size);
+	__put_user_ex(me->sas_ss_size, &frame->uc.uc_stack.ss_size);
 	err |= setup_sigcontext(&frame->uc.uc_mcontext, fp, regs, set->sig[0]);
 	err |= __copy_to_user(&frame->uc.uc_sigmask, set, sizeof(*set));
 
@@ -453,15 +464,12 @@ static int __setup_rt_frame(int sig, struct k_sigaction *ka, siginfo_t *info,
 	   already in userspace.  */
 	/* x86-64 should always use SA_RESTORER. */
 	if (ka->sa.sa_flags & SA_RESTORER) {
-		err |= __put_user(ka->sa.sa_restorer, &frame->pretcode);
+		__put_user_ex(ka->sa.sa_restorer, &frame->pretcode);
 	} else {
 		/* could use a vstub here */
 		return -EFAULT;
 	}
 
-	if (err)
-		return -EFAULT;
-
 	/* Set up registers for signal handler */
 	regs->di = sig;
 	/* In case the signal handler was declared without prototypes */
@@ -479,7 +487,8 @@ static int __setup_rt_frame(int sig, struct k_sigaction *ka, siginfo_t *info,
 	   even if the handler happens to be interrupting 32-bit code. */
 	regs->cs = __USER_CS;
 
-	return 0;
+	__put_user_ex_catch();
+	return err;
 }
 #endif /* CONFIG_X86_32 */
 
@@ -511,31 +520,34 @@ sys_sigaction(int sig, const struct old_sigaction __user *act,
 	struct k_sigaction new_ka, old_ka;
 	int ret;
 
+	__put_user_ex_try(&ret, -EFAULT);
+	__get_user_ex_try(&ret, -EFAULT);
+
 	if (act) {
 		old_sigset_t mask;
 
-		if (!access_ok(VERIFY_READ, act, sizeof(*act)) ||
-		    __get_user(new_ka.sa.sa_handler, &act->sa_handler) ||
-		    __get_user(new_ka.sa.sa_restorer, &act->sa_restorer))
+		if (!access_ok(VERIFY_READ, act, sizeof(*act)))
 			return -EFAULT;
-
-		__get_user(new_ka.sa.sa_flags, &act->sa_flags);
-		__get_user(mask, &act->sa_mask);
+		__get_user_ex(new_ka.sa.sa_handler, &act->sa_handler);
+		__get_user_ex(new_ka.sa.sa_flags, &act->sa_flags);
+		__get_user_ex(mask, &act->sa_mask);
+		__get_user_ex(new_ka.sa.sa_restorer, &act->sa_restorer);
 		siginitset(&new_ka.sa.sa_mask, mask);
 	}
 
 	ret = do_sigaction(sig, act ? &new_ka : NULL, oact ? &old_ka : NULL);
 
 	if (!ret && oact) {
-		if (!access_ok(VERIFY_WRITE, oact, sizeof(*oact)) ||
-		    __put_user(old_ka.sa.sa_handler, &oact->sa_handler) ||
-		    __put_user(old_ka.sa.sa_restorer, &oact->sa_restorer))
+		if (!access_ok(VERIFY_WRITE, oact, sizeof(*oact)))
 			return -EFAULT;
-
-		__put_user(old_ka.sa.sa_flags, &oact->sa_flags);
-		__put_user(old_ka.sa.sa_mask.sig[0], &oact->sa_mask);
+		__put_user_ex(old_ka.sa.sa_handler, &oact->sa_handler);
+		__put_user_ex(old_ka.sa.sa_flags, &oact->sa_flags);
+		__put_user_ex(old_ka.sa.sa_mask.sig[0], &oact->sa_mask);
+		__put_user_ex(old_ka.sa.sa_restorer, &oact->sa_restorer);
 	}
 
+	__get_user_ex_catch();
+	__put_user_ex_catch();
 	return ret;
 }
 #endif /* CONFIG_X86_32 */
-- 
1.6.0.4


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [RFC -tip 4/4] x86: ia32_signal: use __{get|put}_user exception handling framework
  2008-12-23  5:20 [RFC -tip 0/4] x86: improve uaccess in signal Hiroshi Shimamoto
                   ` (2 preceding siblings ...)
  2008-12-23  5:23 ` [RFC -tip 3/4] x86: signal: use " Hiroshi Shimamoto
@ 2008-12-23  5:24 ` Hiroshi Shimamoto
  3 siblings, 0 replies; 9+ messages in thread
From: Hiroshi Shimamoto @ 2008-12-23  5:24 UTC (permalink / raw)
  To: Ingo Molnar, Thomas Gleixner, H. Peter Anvin; +Cc: linux-kernel

From: Hiroshi Shimamoto <h-shimamoto@ct.jp.nec.com>

Impact: cleanup

Use __{get|put}_user exception handling framework.

Signed-off-by: Hiroshi Shimamoto <h-shimamoto@ct.jp.nec.com>
---
 arch/x86/ia32/ia32_signal.c |  194 ++++++++++++++++++++++++-------------------
 1 files changed, 109 insertions(+), 85 deletions(-)

diff --git a/arch/x86/ia32/ia32_signal.c b/arch/x86/ia32/ia32_signal.c
index b195f85..a3b6918 100644
--- a/arch/x86/ia32/ia32_signal.c
+++ b/arch/x86/ia32/ia32_signal.c
@@ -47,7 +47,9 @@ void signal_fault(struct pt_regs *regs, void __user *frame, char *where);
 
 int copy_siginfo_to_user32(compat_siginfo_t __user *to, siginfo_t *from)
 {
-	int err;
+	int err = 0;
+
+	__put_user_ex_try(&err, -EFAULT);
 
 	if (!access_ok(VERIFY_WRITE, to, sizeof(compat_siginfo_t)))
 		return -EFAULT;
@@ -57,69 +59,74 @@ int copy_siginfo_to_user32(compat_siginfo_t __user *to, siginfo_t *from)
 	   It should never copy any pad contained in the structure
 	   to avoid security leaks, but must copy the generic
 	   3 ints plus the relevant union member.  */
-	err = __put_user(from->si_signo, &to->si_signo);
-	err |= __put_user(from->si_errno, &to->si_errno);
-	err |= __put_user((short)from->si_code, &to->si_code);
+	__put_user_ex(from->si_signo, &to->si_signo);
+	__put_user_ex(from->si_errno, &to->si_errno);
+	__put_user_ex((short)from->si_code, &to->si_code);
 
 	if (from->si_code < 0) {
-		err |= __put_user(from->si_pid, &to->si_pid);
-		err |= __put_user(from->si_uid, &to->si_uid);
-		err |= __put_user(ptr_to_compat(from->si_ptr), &to->si_ptr);
+		__put_user_ex(from->si_pid, &to->si_pid);
+		__put_user_ex(from->si_uid, &to->si_uid);
+		__put_user_ex(ptr_to_compat(from->si_ptr), &to->si_ptr);
 	} else {
 		/*
 		 * First 32bits of unions are always present:
 		 * si_pid === si_band === si_tid === si_addr(LS half)
 		 */
-		err |= __put_user(from->_sifields._pad[0],
+		__put_user_ex(from->_sifields._pad[0],
 				  &to->_sifields._pad[0]);
 		switch (from->si_code >> 16) {
 		case __SI_FAULT >> 16:
 			break;
 		case __SI_CHLD >> 16:
-			err |= __put_user(from->si_utime, &to->si_utime);
-			err |= __put_user(from->si_stime, &to->si_stime);
-			err |= __put_user(from->si_status, &to->si_status);
+			__put_user_ex(from->si_utime, &to->si_utime);
+			__put_user_ex(from->si_stime, &to->si_stime);
+			__put_user_ex(from->si_status, &to->si_status);
 			/* FALL THROUGH */
 		default:
 		case __SI_KILL >> 16:
-			err |= __put_user(from->si_uid, &to->si_uid);
+			__put_user_ex(from->si_uid, &to->si_uid);
 			break;
 		case __SI_POLL >> 16:
-			err |= __put_user(from->si_fd, &to->si_fd);
+			__put_user_ex(from->si_fd, &to->si_fd);
 			break;
 		case __SI_TIMER >> 16:
-			err |= __put_user(from->si_overrun, &to->si_overrun);
-			err |= __put_user(ptr_to_compat(from->si_ptr),
+			__put_user_ex(from->si_overrun, &to->si_overrun);
+			__put_user_ex(ptr_to_compat(from->si_ptr),
 					  &to->si_ptr);
 			break;
 			 /* This is not generated by the kernel as of now.  */
 		case __SI_RT >> 16:
 		case __SI_MESGQ >> 16:
-			err |= __put_user(from->si_uid, &to->si_uid);
-			err |= __put_user(from->si_int, &to->si_int);
+			__put_user_ex(from->si_uid, &to->si_uid);
+			__put_user_ex(from->si_int, &to->si_int);
 			break;
 		}
 	}
+
+	__put_user_ex_catch();
 	return err;
 }
 
 int copy_siginfo_from_user32(siginfo_t *to, compat_siginfo_t __user *from)
 {
-	int err;
+	int err = 0;
 	u32 ptr32;
 
+	__get_user_ex_try(&err, -EFAULT);
+
 	if (!access_ok(VERIFY_READ, from, sizeof(compat_siginfo_t)))
 		return -EFAULT;
 
-	err = __get_user(to->si_signo, &from->si_signo);
-	err |= __get_user(to->si_errno, &from->si_errno);
-	err |= __get_user(to->si_code, &from->si_code);
+	__get_user_ex(to->si_signo, &from->si_signo);
+	__get_user_ex(to->si_errno, &from->si_errno);
+	__get_user_ex(to->si_code, &from->si_code);
 
-	err |= __get_user(to->si_pid, &from->si_pid);
-	err |= __get_user(to->si_uid, &from->si_uid);
-	err |= __get_user(ptr32, &from->si_ptr);
+	__get_user_ex(to->si_pid, &from->si_pid);
+	__get_user_ex(to->si_uid, &from->si_uid);
+	__get_user_ex(ptr32, &from->si_ptr);
 	to->si_ptr = compat_ptr(ptr32);
 
+	__get_user_ex_catch();
 	return err;
 }
 
@@ -143,18 +150,21 @@ asmlinkage long sys32_sigaltstack(const stack_ia32_t __user *uss_ptr,
 				  struct pt_regs *regs)
 {
 	stack_t uss, uoss;
-	int ret;
+	int ret = 0;
 	mm_segment_t seg;
 
+	__put_user_ex_try(&ret, -EFAULT);
+	__get_user_ex_try(&ret, -EFAULT);
+
 	if (uss_ptr) {
 		u32 ptr;
 
 		memset(&uss, 0, sizeof(stack_t));
-		if (!access_ok(VERIFY_READ, uss_ptr, sizeof(stack_ia32_t)) ||
-			    __get_user(ptr, &uss_ptr->ss_sp) ||
-			    __get_user(uss.ss_flags, &uss_ptr->ss_flags) ||
-			    __get_user(uss.ss_size, &uss_ptr->ss_size))
+		if (!access_ok(VERIFY_READ, uss_ptr, sizeof(stack_ia32_t)))
 			return -EFAULT;
+		__get_user_ex(ptr, &uss_ptr->ss_sp);
+		__get_user_ex(uss.ss_flags, &uss_ptr->ss_flags);
+		__get_user_ex(uss.ss_size, &uss_ptr->ss_size);
 		uss.ss_sp = compat_ptr(ptr);
 	}
 	seg = get_fs();
@@ -162,12 +172,16 @@ asmlinkage long sys32_sigaltstack(const stack_ia32_t __user *uss_ptr,
 	ret = do_sigaltstack(uss_ptr ? &uss : NULL, &uoss, regs->sp);
 	set_fs(seg);
 	if (ret >= 0 && uoss_ptr)  {
-		if (!access_ok(VERIFY_WRITE, uoss_ptr, sizeof(stack_ia32_t)) ||
-		    __put_user(ptr_to_compat(uoss.ss_sp), &uoss_ptr->ss_sp) ||
-		    __put_user(uoss.ss_flags, &uoss_ptr->ss_flags) ||
-		    __put_user(uoss.ss_size, &uoss_ptr->ss_size))
+		if (!access_ok(VERIFY_WRITE, uoss_ptr, sizeof(stack_ia32_t)))
 			ret = -EFAULT;
+		__put_user_ex(ptr_to_compat(uoss.ss_sp), &uoss_ptr->ss_sp);
+		__put_user_ex(uoss.ss_flags, &uoss_ptr->ss_flags);
+		__put_user_ex(uoss.ss_size, &uoss_ptr->ss_size);
 	}
+
+	__get_user_ex_catch();
+	__put_user_ex_catch();
+
 	return ret;
 }
 
@@ -175,18 +189,18 @@ asmlinkage long sys32_sigaltstack(const stack_ia32_t __user *uss_ptr,
  * Do a signal return; undo the signal stack.
  */
 #define COPY(x)			{		\
-	err |= __get_user(regs->x, &sc->x);	\
+	__get_user_ex(regs->x, &sc->x);	\
 }
 
 #define COPY_SEG_CPL3(seg)	{			\
 		unsigned short tmp;			\
-		err |= __get_user(tmp, &sc->seg);	\
+		__get_user_ex(tmp, &sc->seg);	\
 		regs->seg = tmp | 3;			\
 }
 
 #define RELOAD_SEG(seg)		{		\
 	unsigned int cur, pre;			\
-	err |= __get_user(pre, &sc->seg);	\
+	__get_user_ex(pre, &sc->seg);	\
 	savesegment(seg, cur);			\
 	pre |= 3;				\
 	if (pre != cur)				\
@@ -204,6 +218,8 @@ static int ia32_restore_sigcontext(struct pt_regs *regs,
 	/* Always make any pending restarted system calls return -EINTR */
 	current_thread_info()->restart_block.fn = do_no_restart_syscall;
 
+	__get_user_ex_try(&err, -EFAULT);
+
 #if DEBUG_SIG
 	printk(KERN_DEBUG "SIG restore_sigcontext: "
 	       "sc=%p err(%x) eip(%x) cs(%x) flg(%x)\n",
@@ -216,7 +232,7 @@ static int ia32_restore_sigcontext(struct pt_regs *regs,
 	 * the handler, but does not clobber them at least in the
 	 * normal case.
 	 */
-	err |= __get_user(gs, &sc->gs);
+	__get_user_ex(gs, &sc->gs);
 	gs |= 3;
 	savesegment(gs, oldgs);
 	if (gs != oldgs)
@@ -233,16 +249,18 @@ static int ia32_restore_sigcontext(struct pt_regs *regs,
 	COPY_SEG_CPL3(cs);
 	COPY_SEG_CPL3(ss);
 
-	err |= __get_user(tmpflags, &sc->flags);
+	__get_user_ex(tmpflags, &sc->flags);
 	regs->flags = (regs->flags & ~FIX_EFLAGS) | (tmpflags & FIX_EFLAGS);
 	/* disable syscall checks */
 	regs->orig_ax = -1;
 
-	err |= __get_user(tmp, &sc->fpstate);
+	__get_user_ex(tmp, &sc->fpstate);
 	buf = compat_ptr(tmp);
 	err |= restore_i387_xstate_ia32(buf);
 
-	err |= __get_user(*pax, &sc->ax);
+	__get_user_ex(*pax, &sc->ax);
+
+	__get_user_ex_catch();
 	return err;
 }
 
@@ -320,36 +338,40 @@ static int ia32_setup_sigcontext(struct sigcontext_ia32 __user *sc,
 {
 	int tmp, err = 0;
 
+	__put_user_ex_try(&err, -EFAULT);
+
 	savesegment(gs, tmp);
-	err |= __put_user(tmp, (unsigned int __user *)&sc->gs);
+	__put_user_ex(tmp, (unsigned int __user *)&sc->gs);
 	savesegment(fs, tmp);
-	err |= __put_user(tmp, (unsigned int __user *)&sc->fs);
+	__put_user_ex(tmp, (unsigned int __user *)&sc->fs);
 	savesegment(ds, tmp);
-	err |= __put_user(tmp, (unsigned int __user *)&sc->ds);
+	__put_user_ex(tmp, (unsigned int __user *)&sc->ds);
 	savesegment(es, tmp);
-	err |= __put_user(tmp, (unsigned int __user *)&sc->es);
-
-	err |= __put_user(regs->di, &sc->di);
-	err |= __put_user(regs->si, &sc->si);
-	err |= __put_user(regs->bp, &sc->bp);
-	err |= __put_user(regs->sp, &sc->sp);
-	err |= __put_user(regs->bx, &sc->bx);
-	err |= __put_user(regs->dx, &sc->dx);
-	err |= __put_user(regs->cx, &sc->cx);
-	err |= __put_user(regs->ax, &sc->ax);
-	err |= __put_user(current->thread.trap_no, &sc->trapno);
-	err |= __put_user(current->thread.error_code, &sc->err);
-	err |= __put_user(regs->ip, &sc->ip);
-	err |= __put_user(regs->cs, (unsigned int __user *)&sc->cs);
-	err |= __put_user(regs->flags, &sc->flags);
-	err |= __put_user(regs->sp, &sc->sp_at_signal);
-	err |= __put_user(regs->ss, (unsigned int __user *)&sc->ss);
-
-	err |= __put_user(ptr_to_compat(fpstate), &sc->fpstate);
+	__put_user_ex(tmp, (unsigned int __user *)&sc->es);
+
+	__put_user_ex(regs->di, &sc->di);
+	__put_user_ex(regs->si, &sc->si);
+	__put_user_ex(regs->bp, &sc->bp);
+	__put_user_ex(regs->sp, &sc->sp);
+	__put_user_ex(regs->bx, &sc->bx);
+	__put_user_ex(regs->dx, &sc->dx);
+	__put_user_ex(regs->cx, &sc->cx);
+	__put_user_ex(regs->ax, &sc->ax);
+	__put_user_ex(current->thread.trap_no, &sc->trapno);
+	__put_user_ex(current->thread.error_code, &sc->err);
+	__put_user_ex(regs->ip, &sc->ip);
+	__put_user_ex(regs->cs, (unsigned int __user *)&sc->cs);
+	__put_user_ex(regs->flags, &sc->flags);
+	__put_user_ex(regs->sp, &sc->sp_at_signal);
+	__put_user_ex(regs->ss, (unsigned int __user *)&sc->ss);
+
+	__put_user_ex(ptr_to_compat(fpstate), &sc->fpstate);
 
 	/* non-iBCS2 extensions.. */
-	err |= __put_user(mask, &sc->oldmask);
-	err |= __put_user(current->thread.cr2, &sc->cr2);
+	__put_user_ex(mask, &sc->oldmask);
+	__put_user_ex(current->thread.cr2, &sc->cr2);
+
+	__put_user_ex_catch();
 
 	return err;
 }
@@ -411,13 +433,14 @@ int ia32_setup_frame(int sig, struct k_sigaction *ka,
 		0x80cd,		/* int $0x80 */
 	};
 
+	__put_user_ex_try(&err, -EFAULT);
+
 	frame = get_sigframe(ka, regs, sizeof(*frame), &fpstate);
 
 	if (!access_ok(VERIFY_WRITE, frame, sizeof(*frame)))
 		return -EFAULT;
 
-	if (__put_user(sig, &frame->sig))
-		return -EFAULT;
+	__put_user_ex(sig, &frame->sig);
 
 	if (ia32_setup_sigcontext(&frame->sc, fpstate, regs, set->sig[0]))
 		return -EFAULT;
@@ -438,15 +461,13 @@ int ia32_setup_frame(int sig, struct k_sigaction *ka,
 		else
 			restorer = &frame->retcode;
 	}
-	err |= __put_user(ptr_to_compat(restorer), &frame->pretcode);
+	__put_user_ex(ptr_to_compat(restorer), &frame->pretcode);
 
 	/*
 	 * These are actually not used anymore, but left because some
 	 * gdb versions depend on them as a marker.
 	 */
-	err |= __put_user(*((u64 *)&code), (u64 *)frame->retcode);
-	if (err)
-		return -EFAULT;
+	__put_user_ex(*((u64 *)&code), (u64 *)frame->retcode);
 
 	/* Set up registers for signal handler */
 	regs->sp = (unsigned long) frame;
@@ -468,7 +489,9 @@ int ia32_setup_frame(int sig, struct k_sigaction *ka,
 	       current->comm, current->pid, frame, regs->ip, frame->pretcode);
 #endif
 
-	return 0;
+	__put_user_ex_catch();
+
+	return err;
 }
 
 int ia32_setup_rt_frame(int sig, struct k_sigaction *ka, siginfo_t *info,
@@ -492,28 +515,30 @@ int ia32_setup_rt_frame(int sig, struct k_sigaction *ka, siginfo_t *info,
 		0,
 	};
 
+	__put_user_ex_try(&err, -EFAULT);
+
 	frame = get_sigframe(ka, regs, sizeof(*frame), &fpstate);
 
 	if (!access_ok(VERIFY_WRITE, frame, sizeof(*frame)))
 		return -EFAULT;
 
-	err |= __put_user(sig, &frame->sig);
-	err |= __put_user(ptr_to_compat(&frame->info), &frame->pinfo);
-	err |= __put_user(ptr_to_compat(&frame->uc), &frame->puc);
+	__put_user_ex(sig, &frame->sig);
+	__put_user_ex(ptr_to_compat(&frame->info), &frame->pinfo);
+	__put_user_ex(ptr_to_compat(&frame->uc), &frame->puc);
 	err |= copy_siginfo_to_user32(&frame->info, info);
 	if (err)
 		return -EFAULT;
 
 	/* Create the ucontext.  */
 	if (cpu_has_xsave)
-		err |= __put_user(UC_FP_XSTATE, &frame->uc.uc_flags);
+		__put_user_ex(UC_FP_XSTATE, &frame->uc.uc_flags);
 	else
-		err |= __put_user(0, &frame->uc.uc_flags);
-	err |= __put_user(0, &frame->uc.uc_link);
-	err |= __put_user(current->sas_ss_sp, &frame->uc.uc_stack.ss_sp);
-	err |= __put_user(sas_ss_flags(regs->sp),
+		__put_user_ex(0, &frame->uc.uc_flags);
+	__put_user_ex(0, &frame->uc.uc_link);
+	__put_user_ex(current->sas_ss_sp, &frame->uc.uc_stack.ss_sp);
+	__put_user_ex(sas_ss_flags(regs->sp),
 			  &frame->uc.uc_stack.ss_flags);
-	err |= __put_user(current->sas_ss_size, &frame->uc.uc_stack.ss_size);
+	__put_user_ex(current->sas_ss_size, &frame->uc.uc_stack.ss_size);
 	err |= ia32_setup_sigcontext(&frame->uc.uc_mcontext, fpstate,
 				     regs, set->sig[0]);
 	err |= __copy_to_user(&frame->uc.uc_sigmask, set, sizeof(*set));
@@ -525,15 +550,13 @@ int ia32_setup_rt_frame(int sig, struct k_sigaction *ka, siginfo_t *info,
 	else
 		restorer = VDSO32_SYMBOL(current->mm->context.vdso,
 					 rt_sigreturn);
-	err |= __put_user(ptr_to_compat(restorer), &frame->pretcode);
+	__put_user_ex(ptr_to_compat(restorer), &frame->pretcode);
 
 	/*
 	 * Not actually used anymore, but left because some gdb
 	 * versions need it.
 	 */
-	err |= __put_user(*((u64 *)&code), (u64 *)frame->retcode);
-	if (err)
-		return -EFAULT;
+	__put_user_ex(*((u64 *)&code), (u64 *)frame->retcode);
 
 	/* Set up registers for signal handler */
 	regs->sp = (unsigned long) frame;
@@ -555,5 +578,6 @@ int ia32_setup_rt_frame(int sig, struct k_sigaction *ka, siginfo_t *info,
 	       current->comm, current->pid, frame, regs->ip, frame->pretcode);
 #endif
 
-	return 0;
+	__put_user_ex_catch();
+	return err;
 }
-- 
1.6.0.4


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [RFC -tip 2/4] x86: uaccess: introduce __{get|put}_user exception handling framework
  2008-12-23  5:22 ` [RFC -tip 2/4] x86: uaccess: introduce __{get|put}_user exception handling framework Hiroshi Shimamoto
@ 2008-12-23  5:38   ` Brian Gerst
  2008-12-23  5:47     ` Hiroshi Shimamoto
  2008-12-23 14:30   ` Ingo Molnar
  1 sibling, 1 reply; 9+ messages in thread
From: Brian Gerst @ 2008-12-23  5:38 UTC (permalink / raw)
  To: Hiroshi Shimamoto
  Cc: Ingo Molnar, Thomas Gleixner, H. Peter Anvin, linux-kernel

On Tue, Dec 23, 2008 at 12:22 AM, Hiroshi Shimamoto
<h-shimamoto@ct.jp.nec.com> wrote:
> From: Hiroshi Shimamoto <h-shimamoto@ct.jp.nec.com>
>
> Impact: introduce new framework
>
> Introduce exception handling framework.
> __{get|put}_user_ex_try() begins exception block and
> __{get|put}_user_ex_catch() ends block and if an exception occurred in this
> block using __{get|put}_user_ex, direct jump to __{get|put}_user_ex_catch()
> and err is set to specified value.

You shouldn't do this.  According to the gcc manual[1], "Speaking of
labels, jumps from one asm to another are not supported. The
compiler's optimizers do not know about these jumps, and therefore
they cannot take account of them when deciding how to optimize."

[1] http://gcc.gnu.org/onlinedocs/gcc-4.3.2/gcc/Extended-Asm.html

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [RFC -tip 2/4] x86: uaccess: introduce __{get|put}_user exception handling framework
  2008-12-23  5:38   ` Brian Gerst
@ 2008-12-23  5:47     ` Hiroshi Shimamoto
  0 siblings, 0 replies; 9+ messages in thread
From: Hiroshi Shimamoto @ 2008-12-23  5:47 UTC (permalink / raw)
  To: Brian Gerst; +Cc: Ingo Molnar, Thomas Gleixner, H. Peter Anvin, linux-kernel

Brian Gerst wrote:
> On Tue, Dec 23, 2008 at 12:22 AM, Hiroshi Shimamoto
> <h-shimamoto@ct.jp.nec.com> wrote:
>> From: Hiroshi Shimamoto <h-shimamoto@ct.jp.nec.com>
>>
>> Impact: introduce new framework
>>
>> Introduce exception handling framework.
>> __{get|put}_user_ex_try() begins exception block and
>> __{get|put}_user_ex_catch() ends block and if an exception occurred in this
>> block using __{get|put}_user_ex, direct jump to __{get|put}_user_ex_catch()
>> and err is set to specified value.
> 
> You shouldn't do this.  According to the gcc manual[1], "Speaking of
> labels, jumps from one asm to another are not supported. The
> compiler's optimizers do not know about these jumps, and therefore
> they cannot take account of them when deciding how to optimize."
> 
> [1] http://gcc.gnu.org/onlinedocs/gcc-4.3.2/gcc/Extended-Asm.html
> 
thanks so much for this information!
I didn't know about this and it's what I want to know, thinking
about this series.

Thanks,
Hiroshi

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [RFC -tip 2/4] x86: uaccess: introduce __{get|put}_user exception handling framework
  2008-12-23  5:22 ` [RFC -tip 2/4] x86: uaccess: introduce __{get|put}_user exception handling framework Hiroshi Shimamoto
  2008-12-23  5:38   ` Brian Gerst
@ 2008-12-23 14:30   ` Ingo Molnar
  2008-12-23 19:59     ` Hiroshi Shimamoto
  1 sibling, 1 reply; 9+ messages in thread
From: Ingo Molnar @ 2008-12-23 14:30 UTC (permalink / raw)
  To: Hiroshi Shimamoto, Linus Torvalds
  Cc: Thomas Gleixner, H. Peter Anvin, linux-kernel, Peter Zijlstra


* Hiroshi Shimamoto <h-shimamoto@ct.jp.nec.com> wrote:

> From: Hiroshi Shimamoto <h-shimamoto@ct.jp.nec.com>
> 
> Impact: introduce new framework
> 
> Introduce exception handling framework.
> __{get|put}_user_ex_try() begins exception block and
> __{get|put}_user_ex_catch() ends block and if an exception occurred in this
> block using __{get|put}_user_ex, direct jump to __{get|put}_user_ex_catch()
> and err is set to specified value.

ha, this tickled ~12 year old memories: back then Linus came up with a 
very, very similar scheme, for user-copy exception handling.

Such a scheme would be elegant, creates more compact code (we can use 
conditional results directly in branch instructions instead of having to 
export them into registers), and it makes sense syntactically, but it 
doesnt work: GCC is free to reorder (or eliminate) basic blocks and these 
labels can lose their relationship.

So this cannot be done via inline assembly right now, it needs some 
compiler help. Sniff :)

	Ingo

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [RFC -tip 2/4] x86: uaccess: introduce __{get|put}_user exception handling framework
  2008-12-23 14:30   ` Ingo Molnar
@ 2008-12-23 19:59     ` Hiroshi Shimamoto
  0 siblings, 0 replies; 9+ messages in thread
From: Hiroshi Shimamoto @ 2008-12-23 19:59 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Linus Torvalds, Thomas Gleixner, H. Peter Anvin, linux-kernel,
	Peter Zijlstra

Ingo Molnar wrote:
> * Hiroshi Shimamoto <h-shimamoto@ct.jp.nec.com> wrote:
> 
>> From: Hiroshi Shimamoto <h-shimamoto@ct.jp.nec.com>
>>
>> Impact: introduce new framework
>>
>> Introduce exception handling framework.
>> __{get|put}_user_ex_try() begins exception block and
>> __{get|put}_user_ex_catch() ends block and if an exception occurred in this
>> block using __{get|put}_user_ex, direct jump to __{get|put}_user_ex_catch()
>> and err is set to specified value.
> 
> ha, this tickled ~12 year old memories: back then Linus came up with a 
> very, very similar scheme, for user-copy exception handling.
> 
> Such a scheme would be elegant, creates more compact code (we can use 
> conditional results directly in branch instructions instead of having to 
> export them into registers), and it makes sense syntactically, but it 
> doesnt work: GCC is free to reorder (or eliminate) basic blocks and these 
> labels can lose their relationship.
> 
> So this cannot be done via inline assembly right now, it needs some 
> compiler help. Sniff :)

thanks for the above explanation.
I felt it's very hard to understand GCC.

Thanks,
Hiroshi

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2008-12-23 19:59 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2008-12-23  5:20 [RFC -tip 0/4] x86: improve uaccess in signal Hiroshi Shimamoto
2008-12-23  5:22 ` [RFC -tip 1/4] x86: uaccess: rename __put_user_u64() to __put_user_asm_u64() Hiroshi Shimamoto
2008-12-23  5:22 ` [RFC -tip 2/4] x86: uaccess: introduce __{get|put}_user exception handling framework Hiroshi Shimamoto
2008-12-23  5:38   ` Brian Gerst
2008-12-23  5:47     ` Hiroshi Shimamoto
2008-12-23 14:30   ` Ingo Molnar
2008-12-23 19:59     ` Hiroshi Shimamoto
2008-12-23  5:23 ` [RFC -tip 3/4] x86: signal: use " Hiroshi Shimamoto
2008-12-23  5:24 ` [RFC -tip 4/4] x86: ia32_signal: " Hiroshi Shimamoto

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.