Set TIF_IRET in more places

This patch adds code to set TIF_IRET in sigsuspend and rt_sigsuspend (since they change registers to invoke signal handlers) and ptrace setregs. This prevents clobbering of %ecx and %edx.

diff --exclude-from=/home/ldb/src/exclude -urNdp --exclude='speedtouch.*' --exclude='atmsar.*' linux-2.5.54/arch/i386/kernel/ptrace.c linux-2.5.54-ldb/arch/i386/kernel/ptrace.c
@@ -74,6 +74,8 @@ static inline int put_stack_long(struct 
 static int putreg(struct task_struct *child,
 	unsigned long regno, unsigned long value)
 {
+	set_tsk_thread_flag(child, TIF_IRET);
+
 	switch (regno >> 2) {
 		case FS:
 			if (value && (value & 3) != 3)

diff --exclude-from=/home/ldb/src/exclude -urNdp --exclude='speedtouch.*' --exclude='atmsar.*' linux-2.5.54/arch/i386/kernel/signal.c linux-2.5.54-ldb/arch/i386/kernel/signal.c
@@ -44,6 +44,7 @@ sys_sigsuspend(int history0, int history
 	spin_unlock_irq(&current->sig->siglock);
 
 	regs->eax = -EINTR;
+	set_thread_flag(TIF_IRET);
 	while (1) {
 		current->state = TASK_INTERRUPTIBLE;
 		schedule();

@@ -73,6 +74,7 @@ sys_rt_sigsuspend(sigset_t *unewset, siz
 	spin_unlock_irq(&current->sig->siglock);
 
 	regs->eax = -EINTR;
+	set_thread_flag(TIF_IRET);	
 	while (1) {
 		current->state = TASK_INTERRUPTIBLE;
 		schedule();

Hmm.. I explicitly thought about signals for sysenter, and I don't think any of these are needed. In particular, here's my logic:

- ptrace only acts on a child that is stopped in the signal handling path, so if signal handling i scorrect, then so is ptrace. So no special case handling needed in ptrace.c

- [rt_]sigsuspend() has two exit cases: (a) the signal handler we invoced, and (b) the point that signal handler will eventually return to (ie the system call return point)

In the eventual return case, we don't care about ecx/edx, since they will have been saved on the stack by the trampoline anyway.

In the signal handler we invoce, we also don't care about ecx/edx, since they aren't part of the calling convention, and will in fact have random values anyway (do_signal() will re-initialize the FP state, but leaves the integer regs untouched.

- note that for normal asynchronous signals, it _is_ important that we return with all registers saved, but right now that is handled by the fact that the signal trampoline we build in do_signal() will always use "int 0x80" for the sys_sigreturn() call, and will thus use "iret" when restoring the registers. The synchronous "[rt_]sigsuspend()" really is a special case in that respect.

So I _think_ these are all unnecessary, but I may have missed something. So patch not applied, but you can certainly convince me otherwise. And even if I'm right, maybe it needs a comment?

Linus

I've looked again at it and it is actually less problematic that I first thought but I still see the following two cases:

1. vfork seems to not set any TIF_ flags so a ptracer setting regs while a vforking task is stopped in ptrace_notify called from vfork would result in clobbered %ecx and %edx.

2. A ptracer could use %ecx or %edx to pass information to signal handlers and this would not work with the current [rt_]sigsuspend.

These only need setting TIF_IRET on ptrace setregs though.

There is also the very small advantage of being able to hardcode SYSENTER_RETURN as the return eip for sysexit if TIF_IRET is set in all the 3 places.

Just to be clear: the vfork syscall does not use sysenter. It's not possible with the current implementation. The magic sysenter wrapping code would have to recognize the syscall and handle it special by saving the return address. From glibc's POV the %edx register would be free but I consider this interface far to fragile.

--------------.                        ,-.            444 Castro Street
Ulrich Drepper \    ,-----------------'   \ Mountain View, CA 94041 USA
Red Hat         `--' drepper at redhat.com `---------------------------

vfork and clone do not work at all with sysenter due to user stack issues. We should document that (it's already de-facto documented in glibc, but it should be explicitly documented somewhere).

Btw, the vfork/clone problems aren't sysenter-specific per se: they are really a generic problem with any non-inlined system call mechanism. In particular, vfork() really cannot afford to have a stack frame.

Yes. Although I don't see why it should matter, really.

Linus

I actually meant CLONE_VFORK, that if used without CLONE_VM can go through sysenter (if CLONE_VFORK is not used TIF_NEED_RESCHED is set so there is no problem).

I also think that vfork() could be used with AT_SYSINFO by switching stacks around the call (with care about recursive vfork and signals calling vfork inside the vfork stack).