Re: [uml-devel] SYSCALL, ptrace and syscall restart breakages (Re: [RFC] weird crap with vdso on uml/i386)

Linus Torvalds
2011-08-22T23:28:34Z

On Mon, Aug 22, 2011 at 3:04 PM, H. Peter Anvin <hpa@zytor.com> wrote:
>
> However, we could just issue a SIGILL or SIGSEGV at this point; the same
> way we would if we got an #UD or #GP fault; SIGILL/#UD would be
> consistent with Intel CPUs here.

Considering that this is not a remotely new issue, and that it has
been around for years without anybody even noticing, I'd really prefer
to just fix things going forwards rather than add any code to actively
break any possible unlucky legacy users.

So I think the "let's fix the vdso case for sysenter" + "let's remove
the 32-bit syscall vdso" is the right solution. If somebody has
hardcoded syscall instructions, or generates them dynamically with
some JIT, that's their problem. We'll continue to support it as well
as we ever have (read: "almost nobody will ever notice").

One thing we *could* do is to just say "we never restart a x86-32
'syscall' instruction at all", and just make such a case return EINTR.
IOW, do something along the lines of the appended pseudo-patch.

Because returning -EINTR is always "almost correct".

Hmm?

Linus

---
diff --git a/arch/x86/kernel/signal.c b/arch/x86/kernel/signal.c
index 54ddaeb221c1..bc1a0f8b2707 100644
--- a/arch/x86/kernel/signal.c
+++ b/arch/x86/kernel/signal.c
@@ -678,6 +678,16 @@ setup_rt_frame(int sig, struct k_sigaction *ka,
siginfo_t *info,
return ret;
}

+static void restart_syscall(struct pt_regs *regs, int orig)
+{
+ if (regs->syscall_using_syscall_insn) {
+ regs->ax = -EINTR;
+ return;
+ }
+ regs->ax = orig;
+ regs->ip -= 2;
+}
+
static int
handle_signal(unsigned long sig, siginfo_t *info, struct k_sigaction *ka,
struct pt_regs *regs)
@@ -701,8 +711,7 @@ handle_signal(unsigned long sig, siginfo_t
*info, struct k_sigaction *ka,
}
/* fallthrough */
case -ERESTARTNOINTR:
- regs->ax = regs->orig_ax;
- regs->ip -= 2;
+ restart_syscall(regs, regs->orig_ax);
break;
}
}
@@ -786,13 +795,11 @@ static void do_signal(struct pt_regs *regs)
case -ERESTARTNOHAND:
case -ERESTARTSYS:
case -ERESTARTNOINTR:
- regs->ax = regs->orig_ax;
- regs->ip -= 2;
+ restart_syscall(regs, regs->orig_ax);
break;

case -ERESTART_RESTARTBLOCK:
- regs->ax = NR_restart_syscall;
- regs->ip -= 2;
+ restart_syscall(regs, NR_restart_syscall);
break;
}
}