Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Sergio Durigan Junior <sergiodj@redhat.com>
To: Ruslan Kabatsayev <b7.10110111@gmail.com>
Cc: GDB Patches <gdb-patches@sourceware.org>
Subject: Re: [PATCH] Improve ptrace-error detection on Linux targets
Date: Mon, 19 Aug 2019 13:47:00 -0000	[thread overview]
Message-ID: <87wof9nu65.fsf@redhat.com> (raw)
In-Reply-To: <CAHEcG97SCuNoLTXG-myf-uow7_W8DTgij-=umxqqvK=xCoDN_w@mail.gmail.com>	(Ruslan Kabatsayev's message of "Mon, 19 Aug 2019 12:09:45 +0300")

On Monday, August 19 2019, Ruslan Kabatsayev wrote:

> Hello Sergio,

Hey Ruslan,

> On Mon, 19 Aug 2019 at 06:29, Sergio Durigan Junior <sergiodj@redhat.com> wrote:
>>
>> In Fedora GDB, we carry the following patch:
>>
>>   https://src.fedoraproject.org/rpms/gdb/blob/master/f/gdb-attach-fail-reasons-5of5.patch
>>
>> Its purpose is to try to detect a specific scenario where SELinux's
>> 'deny_ptrace' option is enabled, which prevents GDB from ptrace'ing in
>> order to debug the inferior (PTRACE_ATTACH and PTRACE_ME will fail
>> with EACCES in this case).
>>
>> I like the idea of improving error detection and providing more
>> information to the user (a simple "Permission denied" can be really
>> frustrating), but I don't fully agree with the way the patch was
>> implemented: it makes GDB link against libselinux only for the sake of
>> consulting the 'deny_ptrace' setting, and then prints a warning if
>> ptrace failed and this setting is on.
>>
>> Instead of printing a very certain warning about a very specific
>> setting, I decided to propose the following patch, which prints a
>> generic warning about a possible situation (i.e., without going
>> bothering to make sure that the situation is actually happening).
>> What this means is that whenever a ptrace error is detected, and if
>> errno is either EACCES or EPERM, a warning will be printed pointing
>> the user to our documentation, where she will find more details about
>> possible restrictions in place against ptrace.
>
> Doesn't it worsen user experience compared to the current Fedora
> patch? Now a user first coming across the problem will have to
> navigate the documentation, then check possible scenarios – instead of
> simply getting a friendly explanation what exactly is wrong.

I agree that it is easier to just print the reason for the failure
directly, instead of pointing to the documentation.  However, the reason
I chose the latter is because it may not always be possible to identify
the reason, and this might confuse the user or lead him to wrong
conclusions.  I'm mostly thinking about the seccomp feature here
(because I have no idea how to check if it's active or not), but I'm
also concerned that it's unreasonable to expect users to have
libselinux-devel (on Fedora) installed.

> Compare this with the patch applied by some Linux distributions
> (CentOS IIRC) to make Bash emit
>
>> bash: myprogram: /lib/ld-linux.so.2: bad ELF interpreter: No such file or directory
>
> when ELF interpreter can't be found. By default, Bash only prints the
> less than helpful message
>
>> bash: myprogram: No such file or directory
>
> Your proposal (if it's intended to replace Fedora's GDB patch) is
> similar to replacing this default with
>
>> bash: myprogram: No such file or directory. There might be other reasons than simply command executable not existing. Please see Bash documentation section XYZ for details.
>
> and adding a section XYZ in Bash docs explaining that the reasons
> could include lacking libraries, broken symlink to the binary, stale
> hash table of PATH lookups, etc.. This is considerably worse than the
> patch with explicit announcement of actual problem.

Well, in this specific case it's probably not difficult to check for the
right cause and print the specific warning.  However, if there were more
than 2 or 3 scenarios where a program couldn't be executed, and if
checking for them involved depending on external libraries and such,
then we'd be talking about a similar situation here.

> So as a user, I disfavor this approach, although it might be more
> maintainable for GDB developers. But, if dependency on libselinux is
> the only objection, then why not simply dlopen it as needed (and
> gracefully handle failure to do so) instead of ELF-level linking to
> it? Then there'll be no hard dependency on libselinux, while user
> experience will not degrade too.

Thanks for expressing your opinion as user, much appreciated!

I had considered dlopen'ing libselinux before, but didn't do that
because of what I mentioned earlier: I think it's not reasonable to
expect that the user will have libselinux-devel installed, and SELinux
is just one of the possible scenarios here.

But your message made me rethink and decide to do the following:

- Try to dlopen libselinux, and check for deny_ptrace if possible.  If
  it works and deny_ptrace is on, emit a specific warning about it.  If
  it doesn't work or if deny_ptrace is off, continue.

- Try to check the contents of /proc/sys/kernel/yama/ptrace_scope.  If
  it works and if ptrace_scope > 0, emit a specific warning about it.
  Otherwise, continue.

- See if I can come up with a way to check the seccomp thing.  If
  possible, emit a specific warning about it.  Otherwise, continue.

- If everything failed, emit a generic warning pointing to the docs.

I hope this is a better approach.

Thanks,

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


  reply	other threads:[~2019-08-19 13:47 UTC|newest]

Thread overview: 98+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-08-19  3:29 Sergio Durigan Junior
2019-08-19  9:10 ` Ruslan Kabatsayev
2019-08-19 13:47   ` Sergio Durigan Junior [this message]
2019-08-19 14:57     ` Ruslan Kabatsayev
2019-08-19 16:30     ` Christian Biesinger via gdb-patches
2019-08-19 17:04       ` Sergio Durigan Junior
2019-08-19 14:33 ` Eli Zaretskii
2019-08-25 22:38   ` Sergio Durigan Junior
2019-08-19 18:26 ` Pedro Alves
2019-08-25 22:40   ` Sergio Durigan Junior
2019-08-26 18:32 ` [PATCH v2] " Sergio Durigan Junior
2019-08-26 18:35   ` Christian Biesinger via gdb-patches
2019-08-26 20:51     ` Sergio Durigan Junior
2019-08-26 18:44   ` Eli Zaretskii
2019-08-29 14:40   ` Pedro Alves
2019-08-29 19:27     ` Sergio Durigan Junior
2019-08-29 19:48       ` Sergio Durigan Junior
2019-08-30 19:03         ` Pedro Alves
2019-08-30 19:51         ` [PATCH] Remove "\nError: " suffix from nat/fork-inferior.c:trace_start_error warning message Sergio Durigan Junior
2019-08-30 19:54           ` Pedro Alves
2019-08-30 21:06             ` Sergio Durigan Junior
2019-08-30 12:45       ` [PATCH v2] Improve ptrace-error detection on Linux targets Pedro Alves
2019-09-04 19:21         ` Sergio Durigan Junior
2019-09-04 19:31         ` Sergio Durigan Junior
2019-09-04 19:58           ` Pedro Alves
2019-09-04 20:21             ` Sergio Durigan Junior
2019-09-04 20:35               ` Pedro Alves
2019-09-04 20:56                 ` Sergio Durigan Junior
2019-09-04 21:23                   ` Pedro Alves
2019-09-04 21:36                     ` Sergio Durigan Junior
2019-09-05 12:19                       ` Pedro Alves
2019-09-05 17:58                         ` Sergio Durigan Junior
2019-08-30 12:47   ` Pedro Alves
2019-08-30 14:07     ` Eli Zaretskii
2019-08-30 19:44       ` Sergio Durigan Junior
2019-09-04 19:54 ` [PATCH v3] " Sergio Durigan Junior
2019-09-05 17:04   ` Eli Zaretskii
2019-09-11  1:11 ` [PATCH v4] " Sergio Durigan Junior
2019-09-12 12:39   ` Eli Zaretskii
2019-09-12 18:29     ` Sergio Durigan Junior
2019-09-24 20:40   ` Tom Tromey
2019-09-25 14:14     ` Sergio Durigan Junior
2019-09-25 22:04       ` Tom Tromey
2019-09-26  4:22         ` Sergio Durigan Junior
2019-09-26  4:22 ` [PATCH v5] " Sergio Durigan Junior
2019-09-26 17:32   ` Tom Tromey
2019-09-26 17:48     ` Pedro Alves
2019-09-26 17:51       ` Sergio Durigan Junior
2019-09-26 18:14         ` Pedro Alves
2019-09-26 18:25           ` Sergio Durigan Junior
2019-09-26 17:50     ` Sergio Durigan Junior
2019-09-26 18:13   ` Pedro Alves
2019-09-26 18:23     ` Sergio Durigan Junior
2020-02-26 20:06   ` [PATCH 0/6] Improve ptrace-error detection Sergio Durigan Junior
2020-02-26 20:06     ` [PATCH 5/6] Document Linux-specific possible ptrace restrictions Sergio Durigan Junior
2020-02-26 21:00       ` Ruslan Kabatsayev
2020-02-26 22:08         ` Sergio Durigan Junior
2020-02-26 20:06     ` [PATCH 3/6] Expand 'fork_inferior' to check whether 'traceme_fun' succeeded Sergio Durigan Junior
2020-02-26 20:06     ` [PATCH 6/6] Fix comment for 'gdb_dlopen' Sergio Durigan Junior
2020-02-26 20:23       ` Christian Biesinger via gdb-patches
2020-02-26 20:49         ` Sergio Durigan Junior
2020-02-28 15:21       ` Tom Tromey
2020-02-28 16:05         ` Sergio Durigan Junior
2020-02-26 20:06     ` [PATCH 4/6] Extend GNU/Linux to check for ptrace error Sergio Durigan Junior
2020-02-26 20:06     ` [PATCH 1/6] Introduce scoped_pipe.h Sergio Durigan Junior
2020-02-28 15:23       ` Tom Tromey
2020-02-28 16:08         ` Sergio Durigan Junior
2020-02-28 18:57           ` Tom Tromey
2020-02-28 19:48             ` Sergio Durigan Junior
2020-02-28 19:20       ` Pedro Alves
2020-02-28 19:47         ` Sergio Durigan Junior
2020-02-28 20:07           ` Pedro Alves
2020-02-26 20:06     ` [PATCH 2/6] Don't reset errno/bfd_error on 'throw_perror_with_name' Sergio Durigan Junior
2020-02-28 15:29       ` Tom Tromey
2020-02-28 16:36         ` Sergio Durigan Junior
2020-02-28 18:58           ` Tom Tromey
2020-02-28 19:50             ` Sergio Durigan Junior
2020-02-28 20:06         ` Pedro Alves
2020-02-28 20:35           ` Sergio Durigan Junior
2020-02-28 21:11             ` Pedro Alves
2020-03-02 20:07               ` Sergio Durigan Junior
2020-02-28 19:49       ` Pedro Alves
2020-02-28 20:01         ` Sergio Durigan Junior
     [not found]     ` <87v9nh3yme.fsf@redhat.com>
2020-03-15  4:21       ` [PATCH 0/6] Improve ptrace-error detection Sergio Durigan Junior
2020-03-15 21:16         ` Kevin Buettner
2020-03-17 15:47     ` [PATCH v2 0/5] " Sergio Durigan Junior
2020-03-17 15:47       ` [PATCH v2 1/5] Introduce scoped_pipe.h Sergio Durigan Junior
2020-03-17 15:47       ` [PATCH v2 2/5] Don't reset errno/bfd_error on 'throw_perror_with_name' Sergio Durigan Junior
2020-03-27 18:20         ` Pedro Alves
2020-03-17 15:47       ` [PATCH v2 3/5] Expand 'fork_inferior' to check whether 'traceme_fun' succeeded Sergio Durigan Junior
2020-03-27  4:14         ` Kevin Buettner
2020-03-27 13:06         ` Pedro Alves
2020-03-17 15:47       ` [PATCH v2 4/5] Extend GNU/Linux to check for ptrace error Sergio Durigan Junior
2020-03-27 15:28         ` Pedro Alves
2020-03-27 17:02         ` Kevin Buettner
2020-03-17 15:47       ` [PATCH v2 5/5] Document Linux-specific possible ptrace restrictions Sergio Durigan Junior
2020-03-20  0:53       ` [PATCH v2 0/5] Improve ptrace-error detection Kevin Buettner
2020-03-24 18:23         ` Sergio Durigan Junior

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87wof9nu65.fsf@redhat.com \
    --to=sergiodj@redhat.com \
    --cc=b7.10110111@gmail.com \
    --cc=gdb-patches@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox