gdbserver + fsgsbase kaputt

gdbserver + fsgsbase kaputt

[Borislav Petkov]

Hi,

so there's a breakage of a use case with gdbserver on fsgsbase machines,
see

https://sourceware.org/bugzilla/show_bug.cgi?id=26804

Tom has an even simpler reproducer:

$ cat test.c
int
main (void)
{
  return 0;
}
$ gcc test.c -m32
$ gdbserver localhost:12345 a.out
... other terminal ...
$ 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 correct output is, of course:

...
[Inferior 1 (process 1860) exited normally]

I tried to bisect this but it led me to:

  b745cfba44c1 ("x86/cpu: Enable FSGSBASE on 64bit by default and add a chicken bit")

which simply enables fsgsbase so I could've made a small mistake in the
bisection.

I say small because booting with "nofsgsbase" cures it so it must be
something fsgsbase + ptrace especially since the symptom is a corrupted
stack canary in %gs...

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: gdbserver + fsgsbase kaputt

[Andy Lutomirski]

On Mon, Jan 11, 2021 at 10:15 AM Borislav Petkov <bp@alien8.de> wrote:
>
> Hi,
>
> so there's a breakage of a use case with gdbserver on fsgsbase machines,
> see
>
> https://sourceware.org/bugzilla/show_bug.cgi?id=26804
>
> Tom has an even simpler reproducer:
>
> $ cat test.c
> int
> main (void)
> {
>   return 0;
> }
> $ gcc test.c -m32
> $ gdbserver localhost:12345 a.out
> ... other terminal ...
> $ 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 correct output is, of course:
>
> ...
> [Inferior 1 (process 1860) exited normally]
>
> I tried to bisect this but it led me to:
>
>   b745cfba44c1 ("x86/cpu: Enable FSGSBASE on 64bit by default and add a chicken bit")
>
> which simply enables fsgsbase so I could've made a small mistake in the
> bisection.
>
> I say small because booting with "nofsgsbase" cures it so it must be
> something fsgsbase + ptrace especially since the symptom is a corrupted
> stack canary in %gs...

Hmm.  Can you try booting with unsafe_fsgsbase and bisecting further?
And maybe send me your test binary?  I tried to reproduce this, but it
worked fine, even if I compile the test program with
-fstack-protector-all.

Off the top of my head, I would have expected this to fix it:

commit 40c45904f818c1f6555294ca27afc5fda4f09e68
Author: Andy Lutomirski <luto@kernel.org>
Date:   Fri Jun 26 10:24:29 2020 -0700

    x86/ptrace: Fix 32-bit PTRACE_SETREGS vs fsbase and gsbase

Re: gdbserver + fsgsbase kaputt

[Borislav Petkov]

On Mon, Jan 11, 2021 at 11:27:38AM -0800, Andy Lutomirski wrote:
> Hmm.  Can you try booting with unsafe_fsgsbase and bisecting further?

Well, that bisection ended in that patch:

# first bad commit: [b745cfba44c152c34363eea9e052367b6b1d652b] x86/cpu: Enable FSGSBASE on 64bit by default and add a chicken bit

so I can't go further.

Or do you mean I should add "unsafe_fsgsbase" to grub cmdline and bisect
with fsgsbase enabled in all test kernels?

> And maybe send me your test binary?

It is trivial:

int
main (void)
{
  return 0;
}

how can that make any difference or are you thinking compiler differences?

Lemme send it to you.

> I tried to reproduce this, but it worked fine, even if I compile the
> test program with -fstack-protector-all.

Hmm.

> Off the top of my head, I would have expected this to fix it:
> 
> commit 40c45904f818c1f6555294ca27afc5fda4f09e68
> Author: Andy Lutomirski <luto@kernel.org>
> Date:   Fri Jun 26 10:24:29 2020 -0700
> 
>     x86/ptrace: Fix 32-bit PTRACE_SETREGS vs fsbase and gsbase

More hmm.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: gdbserver + fsgsbase kaputt

[Andy Lutomirski]

> On Jan 11, 2021, at 12:00 PM, Borislav Petkov <bp@alien8.de> wrote:
> 


> Or do you mean I should add "unsafe_fsgsbase" to grub cmdline and bisect
> with fsgsbase enabled in all test kernels?

Yes. But I can also look myself in a bit.

Re: gdbserver + fsgsbase kaputt

[Andy Lutomirski]

On Mon, Jan 11, 2021 at 1:06 PM Andy Lutomirski <luto@amacapital.net> wrote:
>
>
> > On Jan 11, 2021, at 12:00 PM, Borislav Petkov <bp@alien8.de> wrote:
> >
>
>
> > Or do you mean I should add "unsafe_fsgsbase" to grub cmdline and bisect
> > with fsgsbase enabled in all test kernels?
>
> Yes. But I can also look myself in a bit.
>

Tom, if I reproduce it in an interactive gdb and play a bit, I get:

Program received signal SIGSEGV, Segmentation fault.
0xf7df2cb6 in init_cacheinfo () from target:/lib/libc.so.6
(gdb) p $gs = $gs
$1 = 99
(gdb) si

Program terminated with signal SIGSEGV, Segmentation fault.
The program no longer exists.

That's gdb itself crashing.  Any idea what's wrong?

Re: gdbserver + fsgsbase kaputt

[Tom de Vries]

On 1/12/21 12:40 AM, Andy Lutomirski wrote:
> On Mon, Jan 11, 2021 at 1:06 PM Andy Lutomirski <luto@amacapital.net> wrote:
>>
>>
>>> On Jan 11, 2021, at 12:00 PM, Borislav Petkov <bp@alien8.de> wrote:
>>>
>>
>>
>>> Or do you mean I should add "unsafe_fsgsbase" to grub cmdline and bisect
>>> with fsgsbase enabled in all test kernels?
>>
>> Yes. But I can also look myself in a bit.
>>
> 
> Tom, if I reproduce it in an interactive gdb and play a bit, I get:
> 
> Program received signal SIGSEGV, Segmentation fault.
> 0xf7df2cb6 in init_cacheinfo () from target:/lib/libc.so.6
> (gdb) p $gs = $gs
> $1 = 99
> (gdb) si
> 
> Program terminated with signal SIGSEGV, Segmentation fault.
> The program no longer exists.
> 
> That's gdb itself crashing.  Any idea what's wrong?
> 

The first "Program received signal SIGSEGV, Segmentation fault" means
that gdb intercepts the sigsegv, and allows you to inspect it f.i. by
printing $_siginfo.  The inferior is still live at this point.

Then when trying to continue using si,  the signal is passed on to the
inferior, which means it'll be terminated.

AFAIU, gdb has not crashed, and behaves as expected.  See below for a
similar scenario.

Thanks,
- Tom

...
$ cat test2.c
int
main (void)
{
  *((int *)0) = 0;
  return 0;
}
$ gcc test2.c
$ ./a.out
Segmentation fault (core dumped)
$ gdb -q ./a.out
Reading symbols from ./a.out...
(gdb) r
Starting program: /home/vries/a.out

Program received signal SIGSEGV, Segmentation fault.
0x00000000004004a0 in main ()
(gdb) si

Program terminated with signal SIGSEGV, Segmentation fault.
The program no longer exists.
(gdb)
...

Re: gdbserver + fsgsbase kaputt

[Andy Lutomirski]

On Mon, Jan 11, 2021 at 3:52 PM Tom de Vries <tdevries@suse.de> wrote:
>
> On 1/12/21 12:40 AM, Andy Lutomirski wrote:
> > On Mon, Jan 11, 2021 at 1:06 PM Andy Lutomirski <luto@amacapital.net> wrote:
> >>
> >>
> >>> On Jan 11, 2021, at 12:00 PM, Borislav Petkov <bp@alien8.de> wrote:
> >>>
> >>
> >>
> >>> Or do you mean I should add "unsafe_fsgsbase" to grub cmdline and bisect
> >>> with fsgsbase enabled in all test kernels?
> >>
> >> Yes. But I can also look myself in a bit.
> >>
> >
> > Tom, if I reproduce it in an interactive gdb and play a bit, I get:
> >
> > Program received signal SIGSEGV, Segmentation fault.
> > 0xf7df2cb6 in init_cacheinfo () from target:/lib/libc.so.6
> > (gdb) p $gs = $gs
> > $1 = 99
> > (gdb) si
> >
> > Program terminated with signal SIGSEGV, Segmentation fault.
> > The program no longer exists.
> >
> > That's gdb itself crashing.  Any idea what's wrong?
> >
>
> The first "Program received signal SIGSEGV, Segmentation fault" means
> that gdb intercepts the sigsegv, and allows you to inspect it f.i. by
> printing $_siginfo.  The inferior is still live at this point.
>
> Then when trying to continue using si,  the signal is passed on to the
> inferior, which means it'll be terminated.
>
> AFAIU, gdb has not crashed, and behaves as expected.  See below for a
> similar scenario.
>
> Thanks,
> - Tom
>
> ...
> $ cat test2.c
> int
> main (void)
> {
>   *((int *)0) = 0;
>   return 0;
> }
> $ gcc test2.c
> $ ./a.out
> Segmentation fault (core dumped)
> $ gdb -q ./a.out
> Reading symbols from ./a.out...
> (gdb) r
> Starting program: /home/vries/a.out
>
> Program received signal SIGSEGV, Segmentation fault.
> 0x00000000004004a0 in main ()
> (gdb) si
>
> Program terminated with signal SIGSEGV, Segmentation fault.
> The program no longer exists.
> (gdb)
> ...
>

Hah, you're right.  Is there an easy way to tell gdb to suppress the
first signal and try again?

Re: gdbserver + fsgsbase kaputt

[Tom de Vries]

On 1/12/21 4:31 AM, Andy Lutomirski wrote:
> On Mon, Jan 11, 2021 at 3:52 PM Tom de Vries <tdevries@suse.de> wrote:
>>
>> On 1/12/21 12:40 AM, Andy Lutomirski wrote:
>>> On Mon, Jan 11, 2021 at 1:06 PM Andy Lutomirski <luto@amacapital.net> wrote:
>>>>
>>>>
>>>>> On Jan 11, 2021, at 12:00 PM, Borislav Petkov <bp@alien8.de> wrote:
>>>>>
>>>>
>>>>
>>>>> Or do you mean I should add "unsafe_fsgsbase" to grub cmdline and bisect
>>>>> with fsgsbase enabled in all test kernels?
>>>>
>>>> Yes. But I can also look myself in a bit.
>>>>
>>>
>>> Tom, if I reproduce it in an interactive gdb and play a bit, I get:
>>>
>>> Program received signal SIGSEGV, Segmentation fault.
>>> 0xf7df2cb6 in init_cacheinfo () from target:/lib/libc.so.6
>>> (gdb) p $gs = $gs
>>> $1 = 99
>>> (gdb) si
>>>
>>> Program terminated with signal SIGSEGV, Segmentation fault.
>>> The program no longer exists.
>>>
>>> That's gdb itself crashing.  Any idea what's wrong?
>>>
>>
>> The first "Program received signal SIGSEGV, Segmentation fault" means
>> that gdb intercepts the sigsegv, and allows you to inspect it f.i. by
>> printing $_siginfo.  The inferior is still live at this point.
>>
>> Then when trying to continue using si,  the signal is passed on to the
>> inferior, which means it'll be terminated.
>>
>> AFAIU, gdb has not crashed, and behaves as expected.  See below for a
>> similar scenario.
>>
>> Thanks,
>> - Tom
>>
>> ...
>> $ cat test2.c
>> int
>> main (void)
>> {
>>   *((int *)0) = 0;
>>   return 0;
>> }
>> $ gcc test2.c
>> $ ./a.out
>> Segmentation fault (core dumped)
>> $ gdb -q ./a.out
>> Reading symbols from ./a.out...
>> (gdb) r
>> Starting program: /home/vries/a.out
>>
>> Program received signal SIGSEGV, Segmentation fault.
>> 0x00000000004004a0 in main ()
>> (gdb) si
>>
>> Program terminated with signal SIGSEGV, Segmentation fault.
>> The program no longer exists.
>> (gdb)
>> ...
>>
> 
> Hah, you're right.  Is there an easy way to tell gdb to suppress the
> first signal and try again?
> 

Say the signal comes from outside the inferior, f.i. one did "kill -s
SIGSEGV <pid>" or some such.

The command "enqueue-signal 0" ignores the signal, after which you can
continue executing, using f.i. continue or si.

The command "signal 0" is a shorthand for "enqueue-signal 0; continue".

One can also change the handling of the signal in the debug session as a
whole using the command handle, f.i. "handle SIGSEGV nopass" to not pass
the SIGSEGV signal to the inferior.

In this case however, the signal comes from an insn, and we can ignore
it, but stepping further will just regenerate the same signal, unless we
fix the cause.

In the debug scenario below, we:
- fix the cause by overwriting the register containing the invalid
  dereferenced pointer value with the valid pointer value p2
- ignore the signal using "enqueue-signal 0"
- continue execution using "si"

Thanks,
- Tom

...
$ cat test2.c
int a;

int
main (void)
{
  int *p = 0;
  int *p2 = &a;
  *p = 0;
  return 0;
}
$ gcc test2.c -g
$ ./a.out
Segmentation fault (core dumped)
$ objdump -d a.out
  ...
0000000000400497 <main>:
  400497:       55                      push   %rbp
  400498:       48 89 e5                mov    %rsp,%rbp
  40049b:       48 c7 45 f8 00 00 00    movq   $0x0,-0x8(%rbp)
  4004a2:       00
  4004a3:       48 c7 45 f0 2c 10 60    movq   $0x60102c,-0x10(%rbp)
  4004aa:       00
  4004ab:       48 8b 45 f8             mov    -0x8(%rbp),%rax
  4004af:       c7 00 00 00 00 00       movl   $0x0,(%rax)
  4004b5:       b8 00 00 00 00          mov    $0x0,%eax
  4004ba:       5d                      pop    %rbp
  4004bb:       c3                      retq
  4004bc:       0f 1f 40 00             nopl   0x0(%rax)
  ...
$ gdb -q a.out
Reading symbols from a.out...
(gdb) r
Starting program: /home/vries/a.out

Program received signal SIGSEGV, Segmentation fault.
0x00000000004004af in main () at test2.c:8
8         *p = 0;
(gdb) set var $rax = p2
(gdb) queue-signal 0
(gdb) si
9         return 0;
(gdb) p $pc
$1 = (void (*)()) 0x4004b5 <main+30>
(gdb)
...

Re: gdbserver + fsgsbase kaputt

[Bae, Chang Seok]

> On Jan 11, 2021, at 13:06, Andy Lutomirski <luto@amacapital.net> wrote:
> 
>> On Jan 11, 2021, at 12:00 PM, Borislav Petkov <bp@alien8.de> wrote:
>> 
>> Or do you mean I should add "unsafe_fsgsbase" to grub cmdline and bisect
>> with fsgsbase enabled in all test kernels?
> 
> Yes. But I can also look myself in a bit.

I was able to find this patch in that way:

commit 0bf7e460361c703333f3a82e50e7871465fe20f9
Author: Andy Lutomirski <luto@kernel.org>
Date:   Thu May 28 16:13:51 2020 -0400

    x86/process/64: Use FSBSBASE in switch_to() if available


The GDB behavior looks to be different between the two cases -- with vs
without gdb server, when I checked the GS/GSBASE values on the ptrace front.

It set the correct GSBASE (e.g.,=0xf7fcf0c0) of GS=0x63 without running the
server. But GSBASE=0 with the server. When I forced to set the correct base,
it exited normally.

Thanks,
Chang

RE: gdbserver + fsgsbase kaputt

[Metzger, Markus T]

> The GDB behavior looks to be different between the two cases -- with vs
> without gdb server, when I checked the GS/GSBASE values on the ptrace front.

64-bit GDB doesn't support FSGSBASE for 32-bit inferiors and it looks like gdbserver
might not support FSGSBASE, at all.

I had added support for the former as part of the tests I wrote about a year ago [1]
but never submitted the patch.  Was the discussion ever concluded?

The general behavior should be that GDB reads a regset, overwrites the registers it
knows about, and writes it back again to preserve the original values of registers it
doesn't know about.

When I log the values that are read and written for FSGSBASE, however, it looks like
ptrace is returning a non-zero GS_BASE on a read and gdbserver is writing zero on
the next write.

Chang, is that also what you were seeing?

Regards,
Markus.

[1]
https://lkml.org/lkml/2019/11/29/306

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

Re: gdbserver + fsgsbase kaputt

[Andy Lutomirski]

On Tue, Jan 12, 2021 at 3:39 AM Metzger, Markus T
<markus.t.metzger@intel.com> wrote:
>
> > The GDB behavior looks to be different between the two cases -- with vs
> > without gdb server, when I checked the GS/GSBASE values on the ptrace front.
>
> 64-bit GDB doesn't support FSGSBASE for 32-bit inferiors and it looks like gdbserver
> might not support FSGSBASE, at all.
>
> I had added support for the former as part of the tests I wrote about a year ago [1]
> but never submitted the patch.  Was the discussion ever concluded?
>
> The general behavior should be that GDB reads a regset, overwrites the registers it
> knows about, and writes it back again to preserve the original values of registers it
> doesn't know about.
>
> When I log the values that are read and written for FSGSBASE, however, it looks like
> ptrace is returning a non-zero GS_BASE on a read and gdbserver is writing zero on
> the next write.

I instrumented the kernel, and I see:

[   26.990644] getreg: gs_base = 0xf7f8e000
[   26.991694] getreg: GS=0x63, GSBASE=0xf7f8e000
[   26.993117] PTRACE_SETREGS
[   26.993813] putreg: change gsbase from 0xf7f8e000 to 0x0
[   26.995134] putreg: write GS=0x63; old GSBASE=0x0
[   26.996235] PTRACE_SETREGS done

That's gdbserver reading GS and GSBASE and then telling the kernel to
set GS to the same value and GSBASE to 0.

I can come up with horrible kernel hacks to try to work around this,
but gdbserver is really giving the kernel bad instructions here.

--Andy

RE: gdbserver + fsgsbase kaputt

[Metzger, Markus T]

> [   26.990644] getreg: gs_base = 0xf7f8e000
> [   26.991694] getreg: GS=0x63, GSBASE=0xf7f8e000
> [   26.993117] PTRACE_SETREGS
> [   26.993813] putreg: change gsbase from 0xf7f8e000 to 0x0
> [   26.995134] putreg: write GS=0x63; old GSBASE=0x0
> [   26.996235] PTRACE_SETREGS done
> 
> That's gdbserver reading GS and GSBASE and then telling the kernel to
> set GS to the same value and GSBASE to 0.
> 
> I can come up with horrible kernel hacks to try to work around this,
> but gdbserver is really giving the kernel bad instructions here.

I agree that this looks like a GDB bug rather than a kernel bug.  GDB
should preserve the GS_BASE value if it doesn't intend to change it.

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

Re: gdbserver + fsgsbase kaputt

[Andy Lutomirski]

On Tue, Jan 12, 2021 at 9:02 AM Metzger, Markus T
<markus.t.metzger@intel.com> wrote:
>
> > [   26.990644] getreg: gs_base = 0xf7f8e000
> > [   26.991694] getreg: GS=0x63, GSBASE=0xf7f8e000
> > [   26.993117] PTRACE_SETREGS
> > [   26.993813] putreg: change gsbase from 0xf7f8e000 to 0x0
> > [   26.995134] putreg: write GS=0x63; old GSBASE=0x0
> > [   26.996235] PTRACE_SETREGS done
> >
> > That's gdbserver reading GS and GSBASE and then telling the kernel to
> > set GS to the same value and GSBASE to 0.
> >
> > I can come up with horrible kernel hacks to try to work around this,
> > but gdbserver is really giving the kernel bad instructions here.
>
> I agree that this looks like a GDB bug rather than a kernel bug.  GDB
> should preserve the GS_BASE value if it doesn't intend to change it.

Indeed.  But we have this pesky no-userspace-regressions policy in the kernel.

So the question I have is: is this enough of a regression that we need
to hack around it in the kernel?  The specific broken use case seems
quite niche: 64-bit gdbserver targeting 32-bit userspace.  It's taken
two-and-a-half kernel releases for anyone to notice, because sensible
people use plain gdb for local debugging and gdbserver for debugging
VMs, embedded targets, and such.

If we do decide we need to fix this in the kernel, I think we need to
simultaneously add ptrace GPR accessors that work better than the
current mess.  We should have explicit 32-bit GPR access and 64-bit
GPR access, and they should do precisely what is requested without
regard for caller's ABI or the target's CS.L.  And we should add some
API that can be used to look up a segment descriptor so that ptrace()
users can emulate segment writes.  At the same time, we can make
SETREGS so the descriptor lookup when writing GS and totally ignore
the GSBASE value if CS.L == 0, and we backport the entire mess back to
5.9 and ask all debugger maintainers to update their code to use the
new APIs that are kludge-free in future releases.  Everyone wins
except for anyone who wants to understand exactly what the legacy API
does and the suckers who have to write test cases for all of this.

RE: gdbserver + fsgsbase kaputt

[Metzger, Markus T]

> From: Andy Lutomirski <luto@kernel.org>
> 
> On Tue, Jan 12, 2021 at 9:02 AM Metzger, Markus T
> <markus.t.metzger@intel.com> wrote:
> >
> > > [   26.990644] getreg: gs_base = 0xf7f8e000
> > > [   26.991694] getreg: GS=0x63, GSBASE=0xf7f8e000
> > > [   26.993117] PTRACE_SETREGS
> > > [   26.993813] putreg: change gsbase from 0xf7f8e000 to 0x0
> > > [   26.995134] putreg: write GS=0x63; old GSBASE=0x0
> > > [   26.996235] PTRACE_SETREGS done
> > >
> > > That's gdbserver reading GS and GSBASE and then telling the kernel to
> > > set GS to the same value and GSBASE to 0.
> > >
> > > I can come up with horrible kernel hacks to try to work around this,
> > > but gdbserver is really giving the kernel bad instructions here.
> >
> > I agree that this looks like a GDB bug rather than a kernel bug.  GDB
> > should preserve the GS_BASE value if it doesn't intend to change it.
> 
> Indeed.  But we have this pesky no-userspace-regressions policy in the kernel.
> 
> So the question I have is: is this enough of a regression that we need
> to hack around it in the kernel?  The specific broken use case seems
> quite niche: 64-bit gdbserver targeting 32-bit userspace.  It's taken
> two-and-a-half kernel releases for anyone to notice, because sensible
> people use plain gdb for local debugging and gdbserver for debugging
> VMs, embedded targets, and such.

IMHO I'd just fix GDB and leave it at that.  The kernel changes exposed a bug
 in gdbserver.  Tom already submitted a fix.

I'm wondering why we're considering some ugly hacks in the kernel for what's
obviously a bug in user-space, yet ignore changes to GDB functionality (for changing
FS/GS from within GDB) we had been discussing a year ago.

Regards,
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