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

Sergio Durigan Junior
2019-09-05T17:58:35Z

On Thursday, September 05 2019, Pedro Alves wrote:

> 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...

OK, then.

> 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.

Ah, I didn't remember the exact reason, thanks for refreshing my
memory.

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

Fair enough.

> 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.

Funny, at first (when I was trying out the patch) I noticed that this
ptrace call was failing, but thought that, because it is being called
inside the child, it would be good if we could pass errno back to the
parent, as you said. But then I just decided to go the easy way and
call trace_start_error_with_name directly there.

I'll look into passing the errno back to the parent via pipes, as
suggested.

>> 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?

"on the target system" can be confusing for newcomers. But sure, I can
certainly find a way to write the warning in a non-confusing way. I'm
just thinking about the extra work involved to implement this. I'll
design something here and send a v4 when ready.

Thanks,

--
Sergio
GPG key ID: 237A 54B1 0287 28BF 00EF 31F4 D0EB 7628 65FC 5E36
Please send encrypted e-mail if possible