Roland McGrath
2009-09-23 00:46:51 UTC
The 32-bit ptrace syscall on a 64-bit kernel (32-bit debugger on
32-bit task) behaves differently than a native 32-bit kernel. When
setting a register state of orig_eax>=0 and eax=-ERESTART* when the
debugged task is NOT on its way out of a 32-bit syscall, the task will
fail to do the syscall restart logic that it should do.
Test case available at http://sources.redhat.com/cgi-bin/cvsweb.cgi/~checkout~/tests/ptrace-tests/tests/erestartsys-trap.c?cvsroot=systemtap
This happens because the 32-bit ptrace syscall sets eax=0xffffffff
when it sets orig_eax>=0. The resuming task will not sign-extend this
for the -ERESTART* check because TS_COMPAT is not set. (So the task
thinks it is restarting after a 64-bit syscall, not a 32-bit one.)
The fix is to have 32-bit ptrace calls sign-extend eax when orig_eax>=0.
The long comment in the change explains the scenarios and caveats fully.
Reported-by: ***@redhat.com
Signed-off-by: Roland McGrath <***@redhat.com>
Reviewed-by: Oleg Nesterov <***@redhat.com>
---
arch/x86/kernel/ptrace.c | 55 +++++++++++++++++++++++++++++++++++++++++++++-
1 files changed, 54 insertions(+), 1 deletions(-)
diff --git a/arch/x86/kernel/ptrace.c b/arch/x86/kernel/ptrace.c
index 8d7d5c9..ecb7a49 100644
--- a/arch/x86/kernel/ptrace.c
+++ b/arch/x86/kernel/ptrace.c
@@ -1120,7 +1120,6 @@ static int putreg32(struct task_struct *child, unsigned regno, u32 value)
R32(edi, di);
R32(esi, si);
R32(ebp, bp);
- R32(eax, ax);
R32(eip, ip);
R32(esp, sp);
@@ -1130,6 +1129,60 @@ static int putreg32(struct task_struct *child, unsigned regno, u32 value)
* causes (long)orig_ax < 0 tests to fire correctly.
*/
regs->orig_ax = (long) (s32) value;
+
+ /*
+ * Whenever setting orig_eax to indicate a system call in
+ * progress, make sure an eax value set by the debugger gets
+ * sign-extended so that any ax == -ERESTART* tests fire
+ * correctly.
+ *
+ * When those tests (in handle_signal) are done directly
+ * after an actual 32-bit syscall, then TS_COMPAT is set and
+ * so syscall_get_error() does sign-extension. However, the
+ * debugger sometimes saves that state and then restores it
+ * later with the intent of picking up the old thread state
+ * that can be about to do syscall restart.
+ *
+ * When it's a 32-bit debugger, that truncates ax to 32 bits.
+ * If the debugger restores thread state and resumes after a
+ * ptrace stop when the child was not doing a new syscall, it
+ * will not have TS_COMPAT set to make syscall_get_error()
+ * notice and do the sign-extension.
+ *
+ * We can't have syscall_get_error() always sign-extend,
+ * since that's wrong for 64-bit syscalls. We want it to
+ * check TS_COMPAT rather than TIF_IA32 to avoid a false
+ * positive in the oddball case of a 32-bit task doing a
+ * syscall from a 64-bit code segment. In the "restored
+ * thread state" case, it has no way to know whether the
+ * restored state refers to a 32-bit or 64-bit syscall.
+ *
+ * So we can't win 'em all. We assume that if you are using
+ * a 32-bit debugger, you don't really care about arcane
+ * interference with a child trying to use 64-bit syscalls.
+ * (Just use a 64-bit debugger on it instead!) What we do
+ * here makes a 32-bit debugger fiddling a 32-bit task
+ * consistent with what happens on a native 32-bit kernel.
+ *
+ * NOTE! Since we have no similar logic in putreg(), we
+ * just expect a 64-bit debugger to save/restore the full
+ * 64 bits. If a 64-bit debugger were to treat a 32-bit
+ * task differently and save/restore only 32 bits per
+ * register, it would have to grok orig_eax >= 0 and know
+ * to sign-extend its saved eax when setting it as 64 bits.
+ */
+ if (regs->orig_ax >= 0)
+ regs->ax = (long) (s32) regs->ax;
+ break;
+
+ case offsetof(struct user32, regs.eax):
+ /*
+ * As above, for either order of setting both ax and orig_ax.
+ */
+ if (regs->orig_ax >= 0)
+ regs->ax = (long) (s32) value;
+ else
+ regs->ax = value;
break;
case offsetof(struct user32, regs.eflags):
32-bit task) behaves differently than a native 32-bit kernel. When
setting a register state of orig_eax>=0 and eax=-ERESTART* when the
debugged task is NOT on its way out of a 32-bit syscall, the task will
fail to do the syscall restart logic that it should do.
Test case available at http://sources.redhat.com/cgi-bin/cvsweb.cgi/~checkout~/tests/ptrace-tests/tests/erestartsys-trap.c?cvsroot=systemtap
This happens because the 32-bit ptrace syscall sets eax=0xffffffff
when it sets orig_eax>=0. The resuming task will not sign-extend this
for the -ERESTART* check because TS_COMPAT is not set. (So the task
thinks it is restarting after a 64-bit syscall, not a 32-bit one.)
The fix is to have 32-bit ptrace calls sign-extend eax when orig_eax>=0.
The long comment in the change explains the scenarios and caveats fully.
Reported-by: ***@redhat.com
Signed-off-by: Roland McGrath <***@redhat.com>
Reviewed-by: Oleg Nesterov <***@redhat.com>
---
arch/x86/kernel/ptrace.c | 55 +++++++++++++++++++++++++++++++++++++++++++++-
1 files changed, 54 insertions(+), 1 deletions(-)
diff --git a/arch/x86/kernel/ptrace.c b/arch/x86/kernel/ptrace.c
index 8d7d5c9..ecb7a49 100644
--- a/arch/x86/kernel/ptrace.c
+++ b/arch/x86/kernel/ptrace.c
@@ -1120,7 +1120,6 @@ static int putreg32(struct task_struct *child, unsigned regno, u32 value)
R32(edi, di);
R32(esi, si);
R32(ebp, bp);
- R32(eax, ax);
R32(eip, ip);
R32(esp, sp);
@@ -1130,6 +1129,60 @@ static int putreg32(struct task_struct *child, unsigned regno, u32 value)
* causes (long)orig_ax < 0 tests to fire correctly.
*/
regs->orig_ax = (long) (s32) value;
+
+ /*
+ * Whenever setting orig_eax to indicate a system call in
+ * progress, make sure an eax value set by the debugger gets
+ * sign-extended so that any ax == -ERESTART* tests fire
+ * correctly.
+ *
+ * When those tests (in handle_signal) are done directly
+ * after an actual 32-bit syscall, then TS_COMPAT is set and
+ * so syscall_get_error() does sign-extension. However, the
+ * debugger sometimes saves that state and then restores it
+ * later with the intent of picking up the old thread state
+ * that can be about to do syscall restart.
+ *
+ * When it's a 32-bit debugger, that truncates ax to 32 bits.
+ * If the debugger restores thread state and resumes after a
+ * ptrace stop when the child was not doing a new syscall, it
+ * will not have TS_COMPAT set to make syscall_get_error()
+ * notice and do the sign-extension.
+ *
+ * We can't have syscall_get_error() always sign-extend,
+ * since that's wrong for 64-bit syscalls. We want it to
+ * check TS_COMPAT rather than TIF_IA32 to avoid a false
+ * positive in the oddball case of a 32-bit task doing a
+ * syscall from a 64-bit code segment. In the "restored
+ * thread state" case, it has no way to know whether the
+ * restored state refers to a 32-bit or 64-bit syscall.
+ *
+ * So we can't win 'em all. We assume that if you are using
+ * a 32-bit debugger, you don't really care about arcane
+ * interference with a child trying to use 64-bit syscalls.
+ * (Just use a 64-bit debugger on it instead!) What we do
+ * here makes a 32-bit debugger fiddling a 32-bit task
+ * consistent with what happens on a native 32-bit kernel.
+ *
+ * NOTE! Since we have no similar logic in putreg(), we
+ * just expect a 64-bit debugger to save/restore the full
+ * 64 bits. If a 64-bit debugger were to treat a 32-bit
+ * task differently and save/restore only 32 bits per
+ * register, it would have to grok orig_eax >= 0 and know
+ * to sign-extend its saved eax when setting it as 64 bits.
+ */
+ if (regs->orig_ax >= 0)
+ regs->ax = (long) (s32) regs->ax;
+ break;
+
+ case offsetof(struct user32, regs.eax):
+ /*
+ * As above, for either order of setting both ax and orig_ax.
+ */
+ if (regs->orig_ax >= 0)
+ regs->ax = (long) (s32) value;
+ else
+ regs->ax = value;
break;
case offsetof(struct user32, regs.eflags):