2008-12-23 05:21:14

by Hiroshi Shimamoto

[permalink] [raw]
Subject: [RFC -tip 0/4] x86: improve uaccess in signal

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


2008-12-23 05:22:32

by Hiroshi Shimamoto

[permalink] [raw]
Subject: [RFC -tip 1/4] x86: uaccess: rename __put_user_u64() to __put_user_asm_u64()

From: Hiroshi Shimamoto <[email protected]>

Impact: cleanup

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

Signed-off-by: Hiroshi Shimamoto <[email protected]>
---
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

2008-12-23 05:23:15

by Hiroshi Shimamoto

[permalink] [raw]
Subject: [RFC -tip 2/4] x86: uaccess: introduce __{get|put}_user exception handling framework

From: Hiroshi Shimamoto <[email protected]>

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 <[email protected]>
---
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

2008-12-23 05:23:44

by Hiroshi Shimamoto

[permalink] [raw]
Subject: [RFC -tip 3/4] x86: signal: use __{get|put}_user exception handling framework

From: Hiroshi Shimamoto <[email protected]>

Impact: cleanup

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

Signed-off-by: Hiroshi Shimamoto <[email protected]>
---
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

2008-12-23 05:24:21

by Hiroshi Shimamoto

[permalink] [raw]
Subject: [RFC -tip 4/4] x86: ia32_signal: use __{get|put}_user exception handling framework

From: Hiroshi Shimamoto <[email protected]>

Impact: cleanup

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

Signed-off-by: Hiroshi Shimamoto <[email protected]>
---
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

2008-12-23 05:38:19

by Brian Gerst

[permalink] [raw]
Subject: Re: [RFC -tip 2/4] x86: uaccess: introduce __{get|put}_user exception handling framework

On Tue, Dec 23, 2008 at 12:22 AM, Hiroshi Shimamoto
<[email protected]> wrote:
> From: Hiroshi Shimamoto <[email protected]>
>
> 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

2008-12-23 05:48:26

by Hiroshi Shimamoto

[permalink] [raw]
Subject: Re: [RFC -tip 2/4] x86: uaccess: introduce __{get|put}_user exception handling framework

Brian Gerst wrote:
> On Tue, Dec 23, 2008 at 12:22 AM, Hiroshi Shimamoto
> <[email protected]> wrote:
>> From: Hiroshi Shimamoto <[email protected]>
>>
>> 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

2008-12-23 14:31:18

by Ingo Molnar

[permalink] [raw]
Subject: Re: [RFC -tip 2/4] x86: uaccess: introduce __{get|put}_user exception handling framework


* Hiroshi Shimamoto <[email protected]> wrote:

> From: Hiroshi Shimamoto <[email protected]>
>
> 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

2008-12-23 19:59:54

by Hiroshi Shimamoto

[permalink] [raw]
Subject: Re: [RFC -tip 2/4] x86: uaccess: introduce __{get|put}_user exception handling framework

Ingo Molnar wrote:
> * Hiroshi Shimamoto <[email protected]> wrote:
>
>> From: Hiroshi Shimamoto <[email protected]>
>>
>> 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