linux-kernel - Re: [PATCH] Fix get ERESTARTSYS with m32 in x86

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Thu, 1 May 2014 00:35:19 +0800
From:	Hui Zhu <hui_zhu@...tor.com>
To:	Hui Zhu <teawater@...il.com>,
	Mark Kettenis <mark.kettenis@...all.nl>
CC:	"H. Peter Anvin" <hpa@...or.com>, <pinskia@...il.com>,
	Thomas Gleixner <tglx@...utronix.de>,
	Ingo Molnar <mingo@...hat.com>, <x86@...nel.org>,
	<eparis@...hat.com>, <ak@...ux.intel.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"gdb@...rceware.org" <gdb@...rceware.org>,
	Pedro Alves <palves@...hat.com>
Subject: 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 <mark.kettenis@...all.nl> wrote:
 >>> Date: Tue, 29 Apr 2014 22:10:15 -0700
 >>> From: "H. Peter Anvin" <hpa@...or.com>
 >>>
 >>> 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 <hui@...esourcery.com>
---
--- 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
      }


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/