[PATCH][gdb/server] Don't overwrite fs/gs_base with -m32

[PATCH][gdb/server] Don't overwrite fs/gs_base with -m32

[Tom de Vries]

Hi,

Consider a minimal test-case test.c:
...
int main (void) { return 0; }
...
compiled with -m32:
...
$ gcc test.c -m32
...

When running the exec using gdbserver on openSUSE Factory (currently running a
linux kernel version 5.10.5):
...
$ gdbserver localhost:12345 a.out
...
to which we connect in a gdb session, we run into a segfault in the inferior:
...
$ gdb -batch -q -ex "target remote localhost:12345" -ex continue
Program received signal SIGSEGV, Segmentation fault.
0xf7dd8bd2 in init_cacheinfo () at ../sysdeps/x86/cacheinfo.c:761
...

The segfault is caused by gdbserver overwriting $gs_base with 0 using
PTRACE_SETREGS.  After it is overwritten, the next use of $gs in the inferior
will trigger the segfault.

Before linux kernel version 5.9, the value used by PTRACE_SETREGS for $gs_base
was ignored, but starting version 5.9, the linux kernel has support for
intel architecture extension FSGSBASE, which allows users to modify $gs_base,
and consequently PTRACE_SETREGS can no longer ignore the $gs_base value.

The overwrite of $gs_base with 0 is done by a memset in x86_fill_gregset,
which was added in commit 9e0aa64f551 "Fix gdbserver qGetTLSAddr for
x86_64 -m32".  The memset intends to zero-extend 32-bit registers that are
tracked in the regcache to 64-bit when writing them into the PTRACE_SETREGS
data argument.  But in addition, it overwrites  other registers that are
not tracked in the regcache, such as $gs_base.

Fix the segfault by redoing the fix from commit 9e0aa64f551 in minimal form.

Tested on x86_64-linux:
- openSUSE Leap 15.2 (using kernel version 5.3.18):
  - native
  - gdbserver -m32
  - -m32
- openSUSE Factory (using kernel version 5.10.5):
  - native
  - m32

Any comments?

Thanks,
- Tom

[gdb/server] Don't overwrite fs/gs_base with -m32

gdbserver/ChangeLog:

2021-01-19  Tom de Vries  <tdevries@suse.de>

	* linux-x86-low.cc (collect_register_i386): New function.
	(x86_fill_gregset):  Remove memset.  Use collect_register_i386.

---
 gdbserver/linux-x86-low.cc | 55 +++++++++++++++++++++++++++-------------------
 1 file changed, 33 insertions(+), 22 deletions(-)

diff --git a/gdbserver/linux-x86-low.cc b/gdbserver/linux-x86-low.cc
index 35dfdd7e0f3..d3273a18f47 100644
--- a/gdbserver/linux-x86-low.cc
+++ b/gdbserver/linux-x86-low.cc
@@ -397,6 +397,35 @@ x86_target::low_cannot_fetch_register (int regno)
   return regno >= I386_NUM_REGS;
 }
 
+static void
+collect_register_i386 (struct regcache *regcache, int regno, void *buf)
+{
+  collect_register (regcache, regno, buf);
+
+#ifdef __x86_64__
+  /* In case of x86_64 -m32, collect_register only writes 4 bytes, but the
+     space reserved in buf for the register is 8 bytes.  Make sure the entire
+     reserved space is initialized.  */
+
+  gdb_assert (register_size (regcache->tdesc, regno) == 4);
+
+  if (regno == RAX)
+    {
+      /* Sign extend EAX value to avoid potential syscall restart
+	 problems.
+
+	 See amd64_linux_collect_native_gregset() in
+	 gdb/amd64-linux-nat.c for a detailed explanation.  */
+      *(int64_t *) buf = *(int32_t *) buf;
+    }
+  else
+    {
+      /* Zero-extend.  */
+      *(uint64_t *) buf = *(uint32_t *) buf;
+    }
+#endif
+}
+
 static void
 x86_fill_gregset (struct regcache *regcache, void *buf)
 {
@@ -411,32 +440,14 @@ x86_fill_gregset (struct regcache *regcache, void *buf)
 
       return;
     }
-
-  /* 32-bit inferior registers need to be zero-extended.
-     Callers would read uninitialized memory otherwise.  */
-  memset (buf, 0x00, X86_64_USER_REGS * 8);
 #endif
 
   for (i = 0; i < I386_NUM_REGS; i++)
-    collect_register (regcache, i, ((char *) buf) + i386_regmap[i]);
-
-  collect_register_by_name (regcache, "orig_eax",
-			    ((char *) buf) + ORIG_EAX * REGSIZE);
+    collect_register_i386 (regcache, i, ((char *) buf) + i386_regmap[i]);
 
-#ifdef __x86_64__
-  /* Sign extend EAX value to avoid potential syscall restart
-     problems. 
-
-     See amd64_linux_collect_native_gregset() in gdb/amd64-linux-nat.c
-     for a detailed explanation.  */
-  if (register_size (regcache->tdesc, 0) == 4)
-    {
-      void *ptr = ((gdb_byte *) buf
-		   + i386_regmap[find_regno (regcache->tdesc, "eax")]);
-
-      *(int64_t *) ptr = *(int32_t *) ptr;
-    }
-#endif
+  /* Handle ORIG_EAX, which is not in i386_regmap.  */
+  collect_register_i386 (regcache, find_regno (regcache->tdesc, "orig_eax"),
+			 ((char *) buf) + ORIG_EAX * REGSIZE);
 }
 
 static void

Re: [PATCH][gdb/server] Don't overwrite fs/gs_base with -m32

[Simon Marchi]

On 2021-01-20 7:59 a.m., Tom de Vries wrote:
> Hi,
> 
> Consider a minimal test-case test.c:
> ...
> int main (void) { return 0; }
> ...
> compiled with -m32:
> ...
> $ gcc test.c -m32
> ...
> 
> When running the exec using gdbserver on openSUSE Factory (currently running a
> linux kernel version 5.10.5):
> ...
> $ gdbserver localhost:12345 a.out
> ...
> to which we connect in a gdb session, we run into a segfault in the inferior:
> ...
> $ gdb -batch -q -ex "target remote localhost:12345" -ex continue
> Program received signal SIGSEGV, Segmentation fault.
> 0xf7dd8bd2 in init_cacheinfo () at ../sysdeps/x86/cacheinfo.c:761
> ...
> 
> The segfault is caused by gdbserver overwriting $gs_base with 0 using
> PTRACE_SETREGS.  After it is overwritten, the next use of $gs in the inferior
> will trigger the segfault.
> 
> Before linux kernel version 5.9, the value used by PTRACE_SETREGS for $gs_base
> was ignored, but starting version 5.9, the linux kernel has support for
> intel architecture extension FSGSBASE, which allows users to modify $gs_base,
> and consequently PTRACE_SETREGS can no longer ignore the $gs_base value.
> 
> The overwrite of $gs_base with 0 is done by a memset in x86_fill_gregset,
> which was added in commit 9e0aa64f551 "Fix gdbserver qGetTLSAddr for
> x86_64 -m32".  The memset intends to zero-extend 32-bit registers that are
> tracked in the regcache to 64-bit when writing them into the PTRACE_SETREGS
> data argument.  But in addition, it overwrites  other registers that are
> not tracked in the regcache, such as $gs_base.
> 
> Fix the segfault by redoing the fix from commit 9e0aa64f551 in minimal form.
> 
> Tested on x86_64-linux:
> - openSUSE Leap 15.2 (using kernel version 5.3.18):
>   - native
>   - gdbserver -m32
>   - -m32
> - openSUSE Factory (using kernel version 5.10.5):
>   - native
>   - m32
> 
> Any comments?
> 
> Thanks,
> - Tom

This looks good as far as I can tell, but please wait for Markus' to chime in as
well.

Simon

RE: [PATCH][gdb/server] Don't overwrite fs/gs_base with -m32

[Metzger, Markus T]

> -----Original Message-----
> From: Simon Marchi <simon.marchi@polymtl.ca>
> On 2021-01-20 7:59 a.m., Tom de Vries wrote:
> > Hi,
> >
> > Consider a minimal test-case test.c:
> > ...
> > int main (void) { return 0; }
> > ...
> > compiled with -m32:
> > ...
> > $ gcc test.c -m32
> > ...
> >
> > When running the exec using gdbserver on openSUSE Factory (currently running a
> > linux kernel version 5.10.5):
> > ...
> > $ gdbserver localhost:12345 a.out
> > ...
> > to which we connect in a gdb session, we run into a segfault in the inferior:
> > ...
> > $ gdb -batch -q -ex "target remote localhost:12345" -ex continue
> > Program received signal SIGSEGV, Segmentation fault.
> > 0xf7dd8bd2 in init_cacheinfo () at ../sysdeps/x86/cacheinfo.c:761
> > ...
> >
> > The segfault is caused by gdbserver overwriting $gs_base with 0 using
> > PTRACE_SETREGS.  After it is overwritten, the next use of $gs in the inferior
> > will trigger the segfault.
> >
> > Before linux kernel version 5.9, the value used by PTRACE_SETREGS for $gs_base
> > was ignored, but starting version 5.9, the linux kernel has support for
> > intel architecture extension FSGSBASE, which allows users to modify $gs_base,
> > and consequently PTRACE_SETREGS can no longer ignore the $gs_base value.
> >
> > The overwrite of $gs_base with 0 is done by a memset in x86_fill_gregset,
> > which was added in commit 9e0aa64f551 "Fix gdbserver qGetTLSAddr for
> > x86_64 -m32".  The memset intends to zero-extend 32-bit registers that are
> > tracked in the regcache to 64-bit when writing them into the PTRACE_SETREGS
> > data argument.  But in addition, it overwrites  other registers that are
> > not tracked in the regcache, such as $gs_base.
> >
> > Fix the segfault by redoing the fix from commit 9e0aa64f551 in minimal form.
> >
> > Tested on x86_64-linux:
> > - openSUSE Leap 15.2 (using kernel version 5.3.18):
> >   - native
> >   - gdbserver -m32
> >   - -m32
> > - openSUSE Factory (using kernel version 5.10.5):
> >   - native
> >   - m32
> >
> > Any comments?
> >
> > Thanks,
> > - Tom
> 
> This looks good as far as I can tell, but please wait for Markus' to chime in as
> well.

Looks good to me, too.

Markus.
Intel Deutschland GmbH
Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Christin Eisenschmid, Gary Kershaw
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928