Re: [PATCH v2] Improve ptrace-error detection on Linux targets

[Pedro Alves <palves@redhat.com>]

On 9/4/19 10:36 PM, Sergio Durigan Junior wrote:
> On Wednesday, September 04 2019, Pedro Alves wrote:
> 
>> On 9/4/19 9:56 PM, Sergio Durigan Junior wrote:
>>>> You could start gdbserver with the restrictions off (like a long
>>>> lived daemon), and then while gdbserver is running enable
>>>> restrictions, I suppose.
>>> Ah, right, I think that would also work.
>>
>> I'm interested in knowing whether the error percolates to
>> the gdb side in a reasonable form.
> 
> The attach error is displayed by GDB, but the information about ptrace
> restrictions is not.
> 
> This is what I see on GDB:
> 
>   (gdb) attach 32378
>   Attaching to process 32378
>   Attaching to process 32378 failed
> 
> And this is what I see on gdbserver:
> 
>   gdbserver: Cannot attach to process 32378: Permission denied (13), the SELinux boolean 'deny_ptrace' is enabled, you can disable this process attach protection by: (gdb) shell sudo setsebool deny_ptrace=0
> 
> 
> I think there's something wrong with the terminal settings because
> newlines aren't being respected here.  But the message is still there,
> and the user can still understand it, I think.

Odd.  We should fix that...

But this made me realize something.  Here:

  static int
  linux_child_function (void *child_stack)
  {
 -  ptrace (PTRACE_TRACEME, 0, (PTRACE_TYPE_ARG3) 0, (PTRACE_TYPE_ARG4) 0);
 +  if (ptrace (PTRACE_TRACEME, 0, (PTRACE_TYPE_ARG3) 0,
 +             (PTRACE_TYPE_ARG4) 0) != 0)
 +    trace_start_error_with_name ("ptrace",
 +                                linux_ptrace_me_fail_reason (errno).c_str ());
 +

This is code that is run by the fork child.  Recall that we 
had introduced trace_start_error_with_name instead of using
perror_with_name / error, because between after fork and before
exec, we can only safely use async-signal-safe functions.

But that linux_ptrace_me_fail_reason call is doing a lot of
non-async-signal safe things...

I think the right approach would be to make the child pass back the errno
value to the parent, and then have the parent, do the dlopens, the
std::string/malloc uses, etc. and print the error messages.

The traditional way to pass data around like this is via a pipe.
darwin-nat.c uses a pipe around fork, see ptrace_fds, though for a
different reason.

> Ideally we'd have GDB also display the ptrace restriction warning, but I
> think that if the user can see that the attach failed, she will likely
> look at what happened with gdbserver, and will see the error there.  

I don't think that assuming that the user has easy/convenient access
to gdbserver's terminal is a great assumption.  Consider IDEs automating
things, or gdbserver really being used as a service in a container, etc.

Note that at least for Yama, you can have ptrace allow PTRACE_ME, while
PTRACE_ATTACH gets refused.  So it's not guaranteed that gdbserver would
exit out early on start up, meaning, I think that handling this
gdbserver + "(gdb) attach" error case isn't being pedantic.

> If
> we were to show the message on GDB, we'd also have to rewrite it in a
> way that explains that the command needs to be run at the target, which
> may confuse things.
Or it may not confuse things?  I don't think that should prevent
improving things.  Surely we can find some way to express the message
without causing confusion.  I'm not sure it does, but if it does,
maybe we can just append a "on the target system" somewhere?

Thanks,
Pedro Alves