Re: [PATCH] Fix get ERESTARTSYS with m32 in x86_64 when debug by GDB

On 05/01/14 00:28, Hui Zhu wrote:
> On Wed, Apr 30, 2014 at 9:35  PM, Mark Kettenis <[email protected]> wrote:
 >>> Date: Tue, 29 Apr 2014 22:10:15 -0700
 >>> From: "H. Peter Anvin" <[email protected]>
 >>>
 >>> On 04/29/2014 10:08 PM, Andrew Pinski wrote:
 >>>>
 >>>> restoring the values is hard since even the ptrace interface does not
 >>>> allow for that.
 >>>>
 >>>
 >>> So that begs the ultimate question, which is: given the fact that there
 >>> is *state missing* from the state vector (this is the core of the
 >>> problem), is there a way we can add that state so that gdb will be able
 >>> to save and restore it?
 >>
 >> Carrying around additional state in GDB is complicated; I'd rather
 >> avoid it.
 >>
 >> arch/x86/kernel/ptrace.c:putreg32() has this bit of code:
 >>
 >>         case offsetof(struct user32, regs.orig_eax):
 >>                 /*
 >>                  * A 32-bit debugger setting orig_eax means to restore
 >>                  * the state of the task restarting a 32-bit syscall.
 >>                  * Make sure we interpret the -ERESTART* codes correctly
 >>                  * in case the task is not actually still sitting at the
 >>                  * exit from a 32-bit syscall with TS_COMPAT still set.
 >>                  */
 >>                 regs->orig_ax = value;
 >>                 if (syscall_get_nr(child, regs) >= 0)
 >> task_thread_info(child)->status |= TS_COMPAT;
 >>                 break;
 >>
 >> which gets used for 32-bit compat ptrace(2).  Perhaps the same logic
 >> should be added to putreg() if the child is a 32-bit process?
 >
 > Make a new patch that add following code to putreg():
 > case offsetof(struct user_regs_struct, orig_ax):
 > /*
 > * A 64-bit debugger setting orig_ax of a 32-bit inferior
 > * means to restore the state of the task restarting a
 > * 32-bit syscall.
 > * Make sure we interpret the -ERESTART* codes correctly
 > * in case the task is not actually still sitting at the
 > * exit from a 32-bit syscall with TS_COMPAT still set.
 > */
 > if (test_ti_thread_flag(task_thread_info(child), TIF_IA32)) {
 > struct pt_regs *regs = task_pt_regs(child);
 > regs->orig_ax = value;
 > if (syscall_get_nr(child, regs) >= 0)
 > task_thread_info(child)->status |= TS_COMPAT;
 > return 0;
 > }
 >
 >>
 >> If (and only if) the goal of that TS_COMPAT flag solely is to trigger
 >> the error code sign-extension in 
arch/x86/asm/syscall.h:syscall_get_error(),
 >> we could work around to problem in GDB by checking "orig_ax" to see if
 >> we're continuing an interrupted system call and sign extend the error
 >> code in the real "eax" register if we are.
 >
 > I will update patch in
 > https://sourceware.org/ml/gdb-patches/2014-04/msg00429.html
 > according to this comments.
 >
 > Thanks,
 > Hui
 >

I sorry that previous patch has some format issue, post a new one.

Signed-off-by: Hui Zhu <[email protected]>
---
--- a/arch/x86/kernel/ptrace.c
+++ b/arch/x86/kernel/ptrace.c
@@ -452,6 +452,23 @@ static int putreg(struct task_struct *ch
          if (child->thread.gs != value)
              return do_arch_prctl(child, ARCH_SET_GS, value);
          return 0;
+    case offsetof(struct user_regs_struct, orig_ax):
+        /*
+         * A 64-bit debugger setting orig_ax of a 32-bit inferior
+         * means to restore the state of the task restarting a
+         * 32-bit syscall.
+         * Make sure we interpret the -ERESTART* codes correctly
+         * in case the task is not actually still sitting at the
+         * exit from a 32-bit syscall with TS_COMPAT still set.
+         */
+        if (test_ti_thread_flag(task_thread_info(child), TIF_IA32)) {
+            struct pt_regs *regs = task_pt_regs(child);
+            regs->orig_ax = value;
+            if (syscall_get_nr(child, regs) >= 0)
+                task_thread_info(child)->status |= TS_COMPAT;
+            return 0;
+        }
+        break;
  #endif
      }