linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [Bug 210655] ptrace.2: documentation is incorrect about access checking threads in same thread group
@ 2020-12-15 17:01 Alejandro Colomar (man-pages)
  2020-12-15 18:31 ` Ted Estes
  2020-12-15 23:07 ` Jann Horn
  0 siblings, 2 replies; 14+ messages in thread
From: Alejandro Colomar (man-pages) @ 2020-12-15 17:01 UTC (permalink / raw)
  To: Pavel Emelyanov, Oleg Nesterov, Andrew Morton, Michael Kerrisk,
	Kees Cook, Jann Horn
  Cc: Ted Estes, linux-man, linux-kernel

Hi,

There's a bug report: https://bugzilla.kernel.org/show_bug.cgi?id=210655

[[
Under "Ptrace access mode checking", the documentation states:
  "1. If the calling thread and the target thread are in the same thread
group, access is always allowed."

This is incorrect. A thread may never attach to another in the same group.

Reference, ptrace_attach()
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/kernel/ptrace.c?h=v5.9.14#n380
]]

I just wanted to make sure that it is a bug in the manual page, and not
in the implementation.


Thanks,

Alex

-- 
Alejandro Colomar
Linux man-pages comaintainer; https://www.kernel.org/doc/man-pages/
http://www.alejandro-colomar.es

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Bug 210655] ptrace.2: documentation is incorrect about access checking threads in same thread group
  2020-12-15 17:01 [Bug 210655] ptrace.2: documentation is incorrect about access checking threads in same thread group Alejandro Colomar (man-pages)
@ 2020-12-15 18:31 ` Ted Estes
  2020-12-15 18:34   ` Alejandro Colomar (man-pages)
  2020-12-15 23:07 ` Jann Horn
  1 sibling, 1 reply; 14+ messages in thread
From: Ted Estes @ 2020-12-15 18:31 UTC (permalink / raw)
  To: Alejandro Colomar (man-pages),
	Pavel Emelyanov, Oleg Nesterov, Andrew Morton, Michael Kerrisk,
	Kees Cook, Jann Horn
  Cc: linux-man, linux-kernel

Per my research on the topic, the error is in the manual page.  The 
behavior of ptrace(2) was intentionally changed to prohibit attaching to 
a thread in the same group.  Apparently, there were a number of 
ill-behaved edge cases.

I found this email thread on the subject: 
https://lkml.org/lkml/2006/8/31/241

Thank you.
--Ted Estes

On 12/15/2020 11:01 AM, Alejandro Colomar (man-pages) wrote:
> Hi,
>
> There's a bug report: https://bugzilla.kernel.org/show_bug.cgi?id=210655
>
> [[
> Under "Ptrace access mode checking", the documentation states:
>    "1. If the calling thread and the target thread are in the same thread
> group, access is always allowed."
>
> This is incorrect. A thread may never attach to another in the same group.
>
> Reference, ptrace_attach()
> https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/kernel/ptrace.c?h=v5.9.14#n380
> ]]
>
> I just wanted to make sure that it is a bug in the manual page, and not
> in the implementation.
>
>
> Thanks,
>
> Alex
>


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Bug 210655] ptrace.2: documentation is incorrect about access checking threads in same thread group
  2020-12-15 18:31 ` Ted Estes
@ 2020-12-15 18:34   ` Alejandro Colomar (man-pages)
  2020-12-15 22:48     ` Alejandro Colomar (man-pages)
  0 siblings, 1 reply; 14+ messages in thread
From: Alejandro Colomar (man-pages) @ 2020-12-15 18:34 UTC (permalink / raw)
  To: Ted Estes, Pavel Emelyanov, Oleg Nesterov, Andrew Morton,
	Michael Kerrisk, Kees Cook, Jann Horn
  Cc: linux-man, linux-kernel

Hi Ted,

On 12/15/20 7:31 PM, Ted Estes wrote:
> Per my research on the topic, the error is in the manual page.  The
> behavior of ptrace(2) was intentionally changed to prohibit attaching to
> a thread in the same group.  Apparently, there were a number of
> ill-behaved edge cases.
> 
> I found this email thread on the subject:
> https://lkml.org/lkml/2006/8/31/241

Thank you for all the details and links!
I'll fix the page.

Thanks,

Alex

> 
> Thank you.
> --Ted Estes
> 
> On 12/15/2020 11:01 AM, Alejandro Colomar (man-pages) wrote:
>> Hi,
>>
>> There's a bug report: https://bugzilla.kernel.org/show_bug.cgi?id=210655
>>
>> [[
>> Under "Ptrace access mode checking", the documentation states:
>>    "1. If the calling thread and the target thread are in the same thread
>> group, access is always allowed."
>>
>> This is incorrect. A thread may never attach to another in the same
>> group.
>>
>> Reference, ptrace_attach()
>> https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/kernel/ptrace.c?h=v5.9.14#n380
>>
>> ]]
>>
>> I just wanted to make sure that it is a bug in the manual page, and not
>> in the implementation.
>>
>>
>> Thanks,
>>
>> Alex
>>
> 

-- 
Alejandro Colomar
Linux man-pages comaintainer; https://www.kernel.org/doc/man-pages/
http://www.alejandro-colomar.es

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Bug 210655] ptrace.2: documentation is incorrect about access checking threads in same thread group
  2020-12-15 18:34   ` Alejandro Colomar (man-pages)
@ 2020-12-15 22:48     ` Alejandro Colomar (man-pages)
  2020-12-16  0:55       ` Linus Torvalds
  0 siblings, 1 reply; 14+ messages in thread
From: Alejandro Colomar (man-pages) @ 2020-12-15 22:48 UTC (permalink / raw)
  To: Ted Estes, Pavel Emelyanov, Oleg Nesterov, Andrew Morton,
	Michael Kerrisk, Kees Cook, Jann Horn
  Cc: linux-man, linux-kernel, Linus Torvalds, Oleg Nesterov,
	Markus Gutschke, Roland McGrath, Andreas Hobein

[CC += Andreas, Linus, Roland, Markus; fixed Oleg]

On 12/15/20 7:34 PM, Alejandro Colomar (man-pages) wrote:
> Hi Ted,
>
> On 12/15/20 7:31 PM, Ted Estes wrote:
>> Per my research on the topic, the error is in the manual page.  The
>> behavior of ptrace(2) was intentionally changed to prohibit attaching to
>> a thread in the same group.  Apparently, there were a number of
>> ill-behaved edge cases.
>>
>> I found this email thread on the subject:
>> https://lkml.org/lkml/2006/8/31/241

Okay, after reading the LKML thread,
the old behavior was removed because it was very buggy.

We have two options now:

1) Remove that paragraph, as if that behavior had never existed.

   If we do this, not much is lost:
   Only _very_ old kernels had that behavior,
   and it's not even advisable to make use of it on those, AFAICS.

2) Add a note to that paragraph, saying that since kernel 2.X.Y?
   the calling thread and the target thread can't be in the same group.

   Cons: That info is unlikely to be useful, and will only add
   a few more lines to a page that is already very long.

3) Suggestions?

I prefer option 1.

I'll add a larger screenshot of the manual page below,
so that readers don't need to read 'man 2 ptrace':

[[
	...

       The algorithm employed for ptrace access mode  checking  deter‐
       mines  whether  the  calling  process is allowed to perform the
       corresponding action on the target process.  (In  the  case  of
       opening  /proc/[pid]  files,  the  "calling process" is the one
       opening the file, and the process with the corresponding PID is
       the "target process".)  The algorithm is as follows:

       1. If  the calling thread and the target thread are in the same
          thread group, access is always allowed.

       2. If the access mode specifies PTRACE_MODE_FSCREDS, then,  for
          the  check  in the next step, employ the caller's filesystem
          UID and GID.  (As noted in  credentials(7),  the  filesystem
          UID and GID almost always have the same values as the corre‐
          sponding effective IDs.)

          Otherwise, the access mode specifies  PTRACE_MODE_REALCREDS,
          so  use  the caller's real UID and GID for the checks in the
          next step.  (Most APIs that check the caller's UID  and  GID
          use   the   effective  IDs.   For  historical  reasons,  the
          PTRACE_MODE_REALCREDS check uses the real IDs instead.)

	...
]]

Any thoughts before I write the patch?

Thanks,

Alex

>
> Thank you for all the details and links!
> I'll fix the page.
>
> Thanks,
>
> Alex
>
>>
>> Thank you.
>> --Ted Estes
>>
>> On 12/15/2020 11:01 AM, Alejandro Colomar (man-pages) wrote:
>>> Hi,
>>>
>>> There's a bug report: https://bugzilla.kernel.org/show_bug.cgi?id=210655
>>>
>>> [[
>>> Under "Ptrace access mode checking", the documentation states:
>>>    "1. If the calling thread and the target thread are in the same
thread
>>> group, access is always allowed."
>>>
>>> This is incorrect. A thread may never attach to another in the same
>>> group.
>>>
>>> Reference, ptrace_attach()
>>>
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/kernel/ptrace.c?h=v5.9.14#n380
>>>
>>> ]]
>>>
>>> I just wanted to make sure that it is a bug in the manual page, and not
>>> in the implementation.
>>>
>>>
>>> Thanks,
>>>
>>> Alex
>>>
>>
>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Bug 210655] ptrace.2: documentation is incorrect about access checking threads in same thread group
  2020-12-15 17:01 [Bug 210655] ptrace.2: documentation is incorrect about access checking threads in same thread group Alejandro Colomar (man-pages)
  2020-12-15 18:31 ` Ted Estes
@ 2020-12-15 23:07 ` Jann Horn
  2020-12-15 23:23   ` Alejandro Colomar (man-pages)
  1 sibling, 1 reply; 14+ messages in thread
From: Jann Horn @ 2020-12-15 23:07 UTC (permalink / raw)
  To: Alejandro Colomar (man-pages)
  Cc: Pavel Emelyanov, Oleg Nesterov, Andrew Morton, Michael Kerrisk,
	Kees Cook, Ted Estes, linux-man, linux-kernel, Jann Horn

Am Tue, Dec 15, 2020 at 06:01:25PM +0100 schrieb Alejandro Colomar (man-pages):
> Hi,
> 
> There's a bug report: https://bugzilla.kernel.org/show_bug.cgi?id=210655
> 
> [[
> Under "Ptrace access mode checking", the documentation states:
>   "1. If the calling thread and the target thread are in the same thread
> group, access is always allowed."
> 
> This is incorrect. A thread may never attach to another in the same group.

No, that is correct. ptrace-mode access checks do always short-circuit for
tasks in the same thread group:

/* Returns 0 on success, -errno on denial. */
static int __ptrace_may_access(struct task_struct *task, unsigned int mode)
{
[...]
        /* May we inspect the given task?
         * This check is used both for attaching with ptrace
         * and for allowing access to sensitive information in /proc.
         *
         * ptrace_attach denies several cases that /proc allows
         * because setting up the necessary parent/child relationship
         * or halting the specified task is impossible.
         */

        /* Don't let security modules deny introspection */
        if (same_thread_group(task, current))
                return 0;
[...]
}

As the comment explains, you can't actually *attach*
to another task in the same thread group; but that's
not because of the ptrace-style access check rules,
but because specifically *attaching* to another task
in the same thread group doesn't work.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Bug 210655] ptrace.2: documentation is incorrect about access checking threads in same thread group
  2020-12-15 23:07 ` Jann Horn
@ 2020-12-15 23:23   ` Alejandro Colomar (man-pages)
  2020-12-15 23:25     ` Alejandro Colomar (man-pages)
  0 siblings, 1 reply; 14+ messages in thread
From: Alejandro Colomar (man-pages) @ 2020-12-15 23:23 UTC (permalink / raw)
  To: Jann Horn
  Cc: Pavel Emelyanov, Oleg Nesterov, Andrew Morton, Michael Kerrisk,
	Kees Cook, Ted Estes, linux-man, linux-kernel, Jann Horn

Hi Jann,

On 12/16/20 12:07 AM, Jann Horn wrote:
> Am Tue, Dec 15, 2020 at 06:01:25PM +0100 schrieb Alejandro Colomar (man-pages):
>> Hi,
>>
>> There's a bug report: https://bugzilla.kernel.org/show_bug.cgi?id=210655
>>
>> [[
>> Under "Ptrace access mode checking", the documentation states:
>>   "1. If the calling thread and the target thread are in the same thread
>> group, access is always allowed."
>>
>> This is incorrect. A thread may never attach to another in the same group.
> 
> No, that is correct. ptrace-mode access checks do always short-circuit for
> tasks in the same thread group:
> 
> /* Returns 0 on success, -errno on denial. */
> static int __ptrace_may_access(struct task_struct *task, unsigned int mode)
> {
> [...]
>         /* May we inspect the given task?
>          * This check is used both for attaching with ptrace
>          * and for allowing access to sensitive information in /proc.
>          *
>          * ptrace_attach denies several cases that /proc allows
>          * because setting up the necessary parent/child relationship
>          * or halting the specified task is impossible.
>          */
> 
>         /* Don't let security modules deny introspection */
>         if (same_thread_group(task, current))
>                 return 0;
> [...]
> }

AFAICS, that code always returns non-zero,
at least when called from ptrace_attach().

As you can see below,
__ptrace_may_access() is called some lines after
the code pointed to by the bug report.


static int ptrace_attach(struct task_struct *task, long request,
			 unsigned long addr,
			 unsigned long flags)
{
[...]
	if (same_thread_group(task, current))
		goto out;

	/*
	 * Protect exec's credential calculations against our interference;
	 * SUID, SGID and LSM creds get determined differently
	 * under ptrace.
	 */
	retval = -ERESTARTNOINTR;
	if (mutex_lock_interruptible(&task->signal->cred_guard_mutex))
		goto out;

	task_lock(task);
	retval = __ptrace_may_access(task, PTRACE_MODE_ATTACH_REALCREDS);
[...]
}


Thanks,

Alex

> 
> As the comment explains, you can't actually *attach*
> to another task in the same thread group; but that's
> not because of the ptrace-style access check rules,
> but because specifically *attaching* to another task
> in the same thread group doesn't work.
> 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Bug 210655] ptrace.2: documentation is incorrect about access checking threads in same thread group
  2020-12-15 23:23   ` Alejandro Colomar (man-pages)
@ 2020-12-15 23:25     ` Alejandro Colomar (man-pages)
  2020-12-16  0:01       ` Jann Horn
  0 siblings, 1 reply; 14+ messages in thread
From: Alejandro Colomar (man-pages) @ 2020-12-15 23:25 UTC (permalink / raw)
  To: Jann Horn
  Cc: Pavel Emelyanov, Oleg Nesterov, Andrew Morton, Michael Kerrisk,
	Kees Cook, Ted Estes, linux-man, linux-kernel, Jann Horn



On 12/16/20 12:23 AM, Alejandro Colomar (man-pages) wrote:
> Hi Jann,
> 
> On 12/16/20 12:07 AM, Jann Horn wrote:
>> Am Tue, Dec 15, 2020 at 06:01:25PM +0100 schrieb Alejandro Colomar (man-pages):
>>> Hi,
>>>
>>> There's a bug report: https://bugzilla.kernel.org/show_bug.cgi?id=210655
>>>
>>> [[
>>> Under "Ptrace access mode checking", the documentation states:
>>>   "1. If the calling thread and the target thread are in the same thread
>>> group, access is always allowed."
>>>
>>> This is incorrect. A thread may never attach to another in the same group.
>>
>> No, that is correct. ptrace-mode access checks do always short-circuit for
>> tasks in the same thread group:
>>
>> /* Returns 0 on success, -errno on denial. */
>> static int __ptrace_may_access(struct task_struct *task, unsigned int mode)
>> {
>> [...]
>>         /* May we inspect the given task?
>>          * This check is used both for attaching with ptrace
>>          * and for allowing access to sensitive information in /proc.
>>          *
>>          * ptrace_attach denies several cases that /proc allows
>>          * because setting up the necessary parent/child relationship
>>          * or halting the specified task is impossible.
>>          */
>>
>>         /* Don't let security modules deny introspection */
>>         if (same_thread_group(task, current))
>>                 return 0;
>> [...]
>> }
> 
> AFAICS, that code always returns non-zero,

Sorry, I should have said "that code never returns 0".

> at least when called from ptrace_attach().
> 
> As you can see below,
> __ptrace_may_access() is called some lines after
> the code pointed to by the bug report.
> 
> 
> static int ptrace_attach(struct task_struct *task, long request,
> 			 unsigned long addr,
> 			 unsigned long flags)
> {
> [...]
> 	if (same_thread_group(task, current))
> 		goto out;
> 
> 	/*
> 	 * Protect exec's credential calculations against our interference;
> 	 * SUID, SGID and LSM creds get determined differently
> 	 * under ptrace.
> 	 */
> 	retval = -ERESTARTNOINTR;
> 	if (mutex_lock_interruptible(&task->signal->cred_guard_mutex))
> 		goto out;
> 
> 	task_lock(task);
> 	retval = __ptrace_may_access(task, PTRACE_MODE_ATTACH_REALCREDS);
> [...]
> }
> 
> 
> Thanks,
> 
> Alex
> 
>>
>> As the comment explains, you can't actually *attach*
>> to another task in the same thread group; but that's
>> not because of the ptrace-style access check rules,
>> but because specifically *attaching* to another task
>> in the same thread group doesn't work.
>>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Bug 210655] ptrace.2: documentation is incorrect about access checking threads in same thread group
  2020-12-15 23:25     ` Alejandro Colomar (man-pages)
@ 2020-12-16  0:01       ` Jann Horn
  2020-12-16  2:21         ` Ted Estes
  0 siblings, 1 reply; 14+ messages in thread
From: Jann Horn @ 2020-12-16  0:01 UTC (permalink / raw)
  To: Alejandro Colomar (man-pages)
  Cc: Jann Horn, Pavel Emelyanov, Oleg Nesterov, Andrew Morton,
	Michael Kerrisk, Kees Cook, Ted Estes, linux-man, linux-kernel

On Wed, Dec 16, 2020 at 12:25 AM Alejandro Colomar (man-pages)
<alx.manpages@gmail.com> wrote:
> On 12/16/20 12:23 AM, Alejandro Colomar (man-pages) wrote:
> > On 12/16/20 12:07 AM, Jann Horn wrote:
> >> Am Tue, Dec 15, 2020 at 06:01:25PM +0100 schrieb Alejandro Colomar (man-pages):
> >>> There's a bug report: https://bugzilla.kernel.org/show_bug.cgi?id=210655
> >>>
> >>> [[
> >>> Under "Ptrace access mode checking", the documentation states:
> >>>   "1. If the calling thread and the target thread are in the same thread
> >>> group, access is always allowed."
> >>>
> >>> This is incorrect. A thread may never attach to another in the same group.
> >>
> >> No, that is correct. ptrace-mode access checks do always short-circuit for
> >> tasks in the same thread group:
> >>
> >> /* Returns 0 on success, -errno on denial. */
> >> static int __ptrace_may_access(struct task_struct *task, unsigned int mode)
> >> {
> >> [...]
> >>         /* May we inspect the given task?
> >>          * This check is used both for attaching with ptrace
> >>          * and for allowing access to sensitive information in /proc.
> >>          *
> >>          * ptrace_attach denies several cases that /proc allows
> >>          * because setting up the necessary parent/child relationship
> >>          * or halting the specified task is impossible.
> >>          */
> >>
> >>         /* Don't let security modules deny introspection */
> >>         if (same_thread_group(task, current))
> >>                 return 0;
> >> [...]
> >> }
> >
> > AFAICS, that code always returns non-zero,
>
> Sorry, I should have said "that code never returns 0".
>
> > at least when called from ptrace_attach().

Yes.

> > As you can see below,
> > __ptrace_may_access() is called some lines after
> > the code pointed to by the bug report.
> >
> >
> > static int ptrace_attach(struct task_struct *task, long request,
> >                        unsigned long addr,
> >                        unsigned long flags)
> > {
> > [...]
> >       if (same_thread_group(task, current))
> >               goto out;
> >
> >       /*
> >        * Protect exec's credential calculations against our interference;
> >        * SUID, SGID and LSM creds get determined differently
> >        * under ptrace.
> >        */
> >       retval = -ERESTARTNOINTR;
> >       if (mutex_lock_interruptible(&task->signal->cred_guard_mutex))
> >               goto out;
> >
> >       task_lock(task);
> >       retval = __ptrace_may_access(task, PTRACE_MODE_ATTACH_REALCREDS);
> > [...]
> > }

I said exactly that in my last mail:

> >> As the comment explains, you can't actually *attach*
> >> to another task in the same thread group; but that's
> >> not because of the ptrace-style access check rules,
> >> but because specifically *attaching* to another task
> >> in the same thread group doesn't work.

As I said, attaching indeed doesn't work. But that's not what "Ptrace
access mode checking" means. As the first sentence of that section
says:

| Various parts of the kernel-user-space API (not just ptrace()
| operations), require so-called "ptrace access mode" checks,
| whose outcome determines whether an operation is
| permitted (or, in a  few cases,  causes  a "read" operation
| to return sanitized data).

You can find these places by grepping for \bptrace_may_access\b -
operations like e.g. the get_robust_list() syscall will always succeed
when inspecting other tasks in the caller's thread group thanks to
this rule.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Bug 210655] ptrace.2: documentation is incorrect about access checking threads in same thread group
  2020-12-15 22:48     ` Alejandro Colomar (man-pages)
@ 2020-12-16  0:55       ` Linus Torvalds
  2020-12-16  7:34         ` Oleg Nesterov
  0 siblings, 1 reply; 14+ messages in thread
From: Linus Torvalds @ 2020-12-16  0:55 UTC (permalink / raw)
  To: Alejandro Colomar (man-pages)
  Cc: Ted Estes, Pavel Emelyanov, Oleg Nesterov, Andrew Morton,
	Michael Kerrisk, Kees Cook, Jann Horn, linux-man, linux-kernel,
	Oleg Nesterov, Markus Gutschke, Roland McGrath, Andreas Hobein

On Tue, Dec 15, 2020 at 2:48 PM Alejandro Colomar (man-pages)
<alx.manpages@gmail.com> wrote:
>
> 1) Remove that paragraph, as if that behavior had never existed.

If it's been 15 years since that paragraph was relevant, I think just
removing it is the right thing to do.

             Linus

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Bug 210655] ptrace.2: documentation is incorrect about access checking threads in same thread group
  2020-12-16  0:01       ` Jann Horn
@ 2020-12-16  2:21         ` Ted Estes
  2020-12-16  2:33           ` Jann Horn
  0 siblings, 1 reply; 14+ messages in thread
From: Ted Estes @ 2020-12-16  2:21 UTC (permalink / raw)
  To: Jann Horn, Alejandro Colomar (man-pages)
  Cc: Jann Horn, Pavel Emelyanov, Oleg Nesterov, Andrew Morton,
	Michael Kerrisk, Kees Cook, linux-man, linux-kernel

On 12/15/2020 6:01 PM, Jann Horn wrote:
> On Wed, Dec 16, 2020 at 12:25 AM Alejandro Colomar (man-pages)
> <alx.manpages@gmail.com> wrote:
>> On 12/16/20 12:23 AM, Alejandro Colomar (man-pages) wrote:
>>> On 12/16/20 12:07 AM, Jann Horn wrote:
>>>> Am Tue, Dec 15, 2020 at 06:01:25PM +0100 schrieb Alejandro Colomar (man-pages):
>>>>> There's a bug report: https://bugzilla.kernel.org/show_bug.cgi?id=210655
>>>>>
>>>>> [[
>>>>> Under "Ptrace access mode checking", the documentation states:
>>>>>    "1. If the calling thread and the target thread are in the same thread
>>>>> group, access is always allowed."
>>>>>
>>>>> This is incorrect. A thread may never attach to another in the same group.
>>>> No, that is correct. ptrace-mode access checks do always short-circuit for
>>>> tasks in the same thread group:
>>>>
>>>> /* Returns 0 on success, -errno on denial. */
>>>> static int __ptrace_may_access(struct task_struct *task, unsigned int mode)
>>>> {
>>>> [...]
>>>>          /* May we inspect the given task?
>>>>           * This check is used both for attaching with ptrace
>>>>           * and for allowing access to sensitive information in /proc.
>>>>           *
>>>>           * ptrace_attach denies several cases that /proc allows
>>>>           * because setting up the necessary parent/child relationship
>>>>           * or halting the specified task is impossible.
>>>>           */
>>>>
>>>>          /* Don't let security modules deny introspection */
>>>>          if (same_thread_group(task, current))
>>>>                  return 0;
>>>> [...]
>>>> }
>>> AFAICS, that code always returns non-zero,
>> Sorry, I should have said "that code never returns 0".
>>
>>> at least when called from ptrace_attach().
> Yes.
>
>>> As you can see below,
>>> __ptrace_may_access() is called some lines after
>>> the code pointed to by the bug report.
>>>
>>>
>>> static int ptrace_attach(struct task_struct *task, long request,
>>>                         unsigned long addr,
>>>                         unsigned long flags)
>>> {
>>> [...]
>>>        if (same_thread_group(task, current))
>>>                goto out;
>>>
>>>        /*
>>>         * Protect exec's credential calculations against our interference;
>>>         * SUID, SGID and LSM creds get determined differently
>>>         * under ptrace.
>>>         */
>>>        retval = -ERESTARTNOINTR;
>>>        if (mutex_lock_interruptible(&task->signal->cred_guard_mutex))
>>>                goto out;
>>>
>>>        task_lock(task);
>>>        retval = __ptrace_may_access(task, PTRACE_MODE_ATTACH_REALCREDS);
>>> [...]
>>> }
> I said exactly that in my last mail:
>
>>>> As the comment explains, you can't actually *attach*
>>>> to another task in the same thread group; but that's
>>>> not because of the ptrace-style access check rules,
>>>> but because specifically *attaching* to another task
>>>> in the same thread group doesn't work.
> As I said, attaching indeed doesn't work. But that's not what "Ptrace
> access mode checking" means. As the first sentence of that section
> says:
>
> | Various parts of the kernel-user-space API (not just ptrace()
> | operations), require so-called "ptrace access mode" checks,
> | whose outcome determines whether an operation is
> | permitted (or, in a  few cases,  causes  a "read" operation
> | to return sanitized data).
>
> You can find these places by grepping for \bptrace_may_access\b -
> operations like e.g. the get_robust_list() syscall will always succeed
> when inspecting other tasks in the caller's thread group thanks to
> this rule.

Ah, yes.  I missed that back reference while trying to digest that 
rather meaty man page.  A grep on the man page source tree does show a 
number of references to "ptrace access mode".

That said, the ptrace(2) man page also directly references the ptrace 
access mode check under both PTRACE_ATTACH and PTACE_SEIZE:

| Permission to perform a PTRACE_ATTACH is governed by a ptrace | access 
mode PTRACE_MODE_ATTACH_REALCREDS check; see below. As confirmed, the 
"same thread group" rule does not apply to either of those operations. A 
re-wording of rule 1 similar to this might help avoid confusion: 1. If 
the calling thread and the target thread are in the same thread group: 
a. For ptrace() called with PTRACE_ATTACH or PTRACE_SEIZE, access is 
NEVER allowed. b. For all other so-called "ptrace access mode checks", 
access is ALWAYS allowed. --Ted


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Bug 210655] ptrace.2: documentation is incorrect about access checking threads in same thread group
  2020-12-16  2:21         ` Ted Estes
@ 2020-12-16  2:33           ` Jann Horn
  2020-12-16  9:22             ` Alejandro Colomar (man-pages)
  0 siblings, 1 reply; 14+ messages in thread
From: Jann Horn @ 2020-12-16  2:33 UTC (permalink / raw)
  To: Ted Estes
  Cc: Alejandro Colomar (man-pages),
	Jann Horn, Pavel Emelyanov, Oleg Nesterov, Andrew Morton,
	Michael Kerrisk, Kees Cook, linux-man, linux-kernel

On Wed, Dec 16, 2020 at 3:21 AM Ted Estes <ted@softwarecrafters.com> wrote:
> On 12/15/2020 6:01 PM, Jann Horn wrote:
> > On Wed, Dec 16, 2020 at 12:25 AM Alejandro Colomar (man-pages)
> > <alx.manpages@gmail.com> wrote:
> >> On 12/16/20 12:23 AM, Alejandro Colomar (man-pages) wrote:
> >>> On 12/16/20 12:07 AM, Jann Horn wrote:
> >>>> As the comment explains, you can't actually *attach*
> >>>> to another task in the same thread group; but that's
> >>>> not because of the ptrace-style access check rules,
> >>>> but because specifically *attaching* to another task
> >>>> in the same thread group doesn't work.
> > As I said, attaching indeed doesn't work. But that's not what "Ptrace
> > access mode checking" means. As the first sentence of that section
> > says:
> >
> > | Various parts of the kernel-user-space API (not just ptrace()
> > | operations), require so-called "ptrace access mode" checks,
> > | whose outcome determines whether an operation is
> > | permitted (or, in a  few cases,  causes  a "read" operation
> > | to return sanitized data).
> >
> > You can find these places by grepping for \bptrace_may_access\b -
> > operations like e.g. the get_robust_list() syscall will always succeed
> > when inspecting other tasks in the caller's thread group thanks to
> > this rule.
>
> Ah, yes.  I missed that back reference while trying to digest that
> rather meaty man page.  A grep on the man page source tree does show a
> number of references to "ptrace access mode".
>
> That said, the ptrace(2) man page also directly references the ptrace
> access mode check under both PTRACE_ATTACH and PTACE_SEIZE:
>
> | Permission to perform a PTRACE_ATTACH is governed by a ptrace | access
> mode PTRACE_MODE_ATTACH_REALCREDS check; see below. As confirmed, the
> "same thread group" rule does not apply to either of those operations. A
> re-wording of rule 1 similar to this might help avoid confusion: 1. If
> the calling thread and the target thread are in the same thread group:
> a. For ptrace() called with PTRACE_ATTACH or PTRACE_SEIZE, access is
> NEVER allowed. b. For all other so-called "ptrace access mode checks",
> access is ALWAYS allowed. --Ted

Yeah, maybe. OTOH I'm not sure whether it really makes sense to
explain this as being part of a security check, or whether it should
be explained separately as a restriction on PTRACE_ATTACH and
PTRACE_SEIZE (with a note like "(irrelevant for ptrace attachment)" on
rule 1). But I don't feel strongly about it either way.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Bug 210655] ptrace.2: documentation is incorrect about access checking threads in same thread group
  2020-12-16  0:55       ` Linus Torvalds
@ 2020-12-16  7:34         ` Oleg Nesterov
  0 siblings, 0 replies; 14+ messages in thread
From: Oleg Nesterov @ 2020-12-16  7:34 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Alejandro Colomar (man-pages),
	Ted Estes, Pavel Emelyanov, Andrew Morton, Michael Kerrisk,
	Kees Cook, Jann Horn, linux-man, linux-kernel, Markus Gutschke,
	Roland McGrath, Andreas Hobein

On 12/15, Linus Torvalds wrote:
>
> On Tue, Dec 15, 2020 at 2:48 PM Alejandro Colomar (man-pages)
> <alx.manpages@gmail.com> wrote:
> >
> > 1) Remove that paragraph, as if that behavior had never existed.
> 
> If it's been 15 years since that paragraph was relevant, I think just
> removing it is the right thing to do.

Agreed.

Oleg.


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Bug 210655] ptrace.2: documentation is incorrect about access checking threads in same thread group
  2020-12-16  2:33           ` Jann Horn
@ 2020-12-16  9:22             ` Alejandro Colomar (man-pages)
  2020-12-17 18:45               ` Eric W. Biederman
  0 siblings, 1 reply; 14+ messages in thread
From: Alejandro Colomar (man-pages) @ 2020-12-16  9:22 UTC (permalink / raw)
  To: Oleg Nesterov, Jann Horn, Ted Estes
  Cc: Jann Horn, Pavel Emelyanov, Andrew Morton, Michael Kerrisk,
	Kees Cook, linux-man, linux-kernel, Thomas Gleixner, Ingo Molnar,
	Peter Zijlstra, Darren Hart

[CC += Thomas, Ingo, Peter, Darren]

Hi Oleg,

On 12/16/20 3:33 AM, Jann Horn wrote:
> On Wed, Dec 16, 2020 at 3:21 AM Ted Estes <ted@softwarecrafters.com> wrote:
>> On 12/15/2020 6:01 PM, Jann Horn wrote:
>>> On Wed, Dec 16, 2020 at 12:25 AM Alejandro Colomar (man-pages)
>>> <alx.manpages@gmail.com> wrote:
>>>> On 12/16/20 12:23 AM, Alejandro Colomar (man-pages) wrote:
>>>>> On 12/16/20 12:07 AM, Jann Horn wrote:
>>>>>> As the comment explains, you can't actually *attach*
>>>>>> to another task in the same thread group; but that's
>>>>>> not because of the ptrace-style access check rules,
>>>>>> but because specifically *attaching* to another task
>>>>>> in the same thread group doesn't work.
>>> As I said, attaching indeed doesn't work. But that's not what "Ptrace
>>> access mode checking" means. As the first sentence of that section
>>> says:
>>>
>>> | Various parts of the kernel-user-space API (not just ptrace()
>>> | operations), require so-called "ptrace access mode" checks,
>>> | whose outcome determines whether an operation is
>>> | permitted (or, in a  few cases,  causes  a "read" operation
>>> | to return sanitized data).
>>>
>>> You can find these places by grepping for \bptrace_may_access\b -
>>> operations like e.g. the get_robust_list() syscall will always succeed
>>> when inspecting other tasks in the caller's thread group thanks to
>>> this rule.
>>
>> Ah, yes.  I missed that back reference while trying to digest that
>> rather meaty man page.  A grep on the man page source tree does show a
>> number of references to "ptrace access mode".
>>
>> That said, the ptrace(2) man page also directly references the ptrace
>> access mode check under both PTRACE_ATTACH and PTACE_SEIZE:
>>
>> | Permission to perform a PTRACE_ATTACH is governed by a ptrace | access
>> mode PTRACE_MODE_ATTACH_REALCREDS check; see below. As confirmed, the
>> "same thread group" rule does not apply to either of those operations. A
>> re-wording of rule 1 similar to this might help avoid confusion: 1. If
>> the calling thread and the target thread are in the same thread group:
>> a. For ptrace() called with PTRACE_ATTACH or PTRACE_SEIZE, access is
>> NEVER allowed. b. For all other so-called "ptrace access mode checks",
>> access is ALWAYS allowed. --Ted
> 
> Yeah, maybe. OTOH I'm not sure whether it really makes sense to
> explain this as being part of a security check, or whether it should
> be explained separately as a restriction on PTRACE_ATTACH and
> PTRACE_SEIZE (with a note like "(irrelevant for ptrace attachment)" on
> rule 1). But I don't feel strongly about it either way.
> 

As you are the maintainer for ptrace,
could you confirm the above from Jan?
And maybe suggest what you would do with the manual page.

I'd like to get confirmation that there are still other functions that
require "ptrace access mode" other than ptrace() itself, where it's
valid that the calling thread and the target thread are in the same group.

Jann noted get_robust_list() as an example, so I CCed futex maintainers.

Thanks,

Alex

-- 
Alejandro Colomar
Linux man-pages comaintainer; https://www.kernel.org/doc/man-pages/
http://www.alejandro-colomar.es

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Bug 210655] ptrace.2: documentation is incorrect about access checking threads in same thread group
  2020-12-16  9:22             ` Alejandro Colomar (man-pages)
@ 2020-12-17 18:45               ` Eric W. Biederman
  0 siblings, 0 replies; 14+ messages in thread
From: Eric W. Biederman @ 2020-12-17 18:45 UTC (permalink / raw)
  To: Alejandro Colomar (man-pages)
  Cc: Oleg Nesterov, Jann Horn, Ted Estes, Jann Horn, Pavel Emelyanov,
	Andrew Morton, Michael Kerrisk, Kees Cook, linux-man,
	linux-kernel, Thomas Gleixner, Ingo Molnar, Peter Zijlstra,
	Darren Hart

"Alejandro Colomar (man-pages)" <alx.manpages@gmail.com> writes:

> [CC += Thomas, Ingo, Peter, Darren]
>
> Hi Oleg,
>
> On 12/16/20 3:33 AM, Jann Horn wrote:
>> On Wed, Dec 16, 2020 at 3:21 AM Ted Estes <ted@softwarecrafters.com> wrote:
>>> On 12/15/2020 6:01 PM, Jann Horn wrote:
>>>> On Wed, Dec 16, 2020 at 12:25 AM Alejandro Colomar (man-pages)
>>>> <alx.manpages@gmail.com> wrote:
>>>>> On 12/16/20 12:23 AM, Alejandro Colomar (man-pages) wrote:
>>>>>> On 12/16/20 12:07 AM, Jann Horn wrote:
>>>>>>> As the comment explains, you can't actually *attach*
>>>>>>> to another task in the same thread group; but that's
>>>>>>> not because of the ptrace-style access check rules,
>>>>>>> but because specifically *attaching* to another task
>>>>>>> in the same thread group doesn't work.
>>>> As I said, attaching indeed doesn't work. But that's not what "Ptrace
>>>> access mode checking" means. As the first sentence of that section
>>>> says:
>>>>
>>>> | Various parts of the kernel-user-space API (not just ptrace()
>>>> | operations), require so-called "ptrace access mode" checks,
>>>> | whose outcome determines whether an operation is
>>>> | permitted (or, in a  few cases,  causes  a "read" operation
>>>> | to return sanitized data).
>>>>
>>>> You can find these places by grepping for \bptrace_may_access\b -
>>>> operations like e.g. the get_robust_list() syscall will always succeed
>>>> when inspecting other tasks in the caller's thread group thanks to
>>>> this rule.
>>>
>>> Ah, yes.  I missed that back reference while trying to digest that
>>> rather meaty man page.  A grep on the man page source tree does show a
>>> number of references to "ptrace access mode".
>>>
>>> That said, the ptrace(2) man page also directly references the ptrace
>>> access mode check under both PTRACE_ATTACH and PTACE_SEIZE:
>>>
>>> | Permission to perform a PTRACE_ATTACH is governed by a ptrace | access
>>> mode PTRACE_MODE_ATTACH_REALCREDS check; see below. As confirmed, the
>>> "same thread group" rule does not apply to either of those operations. A
>>> re-wording of rule 1 similar to this might help avoid confusion: 1. If
>>> the calling thread and the target thread are in the same thread group:
>>> a. For ptrace() called with PTRACE_ATTACH or PTRACE_SEIZE, access is
>>> NEVER allowed. b. For all other so-called "ptrace access mode checks",
>>> access is ALWAYS allowed. --Ted
>> 
>> Yeah, maybe. OTOH I'm not sure whether it really makes sense to
>> explain this as being part of a security check, or whether it should
>> be explained separately as a restriction on PTRACE_ATTACH and
>> PTRACE_SEIZE (with a note like "(irrelevant for ptrace attachment)" on
>> rule 1). But I don't feel strongly about it either way.
>> 
>
> As you are the maintainer for ptrace,
> could you confirm the above from Jan?
> And maybe suggest what you would do with the manual page.
>
> I'd like to get confirmation that there are still other functions that
> require "ptrace access mode" other than ptrace() itself, where it's
> valid that the calling thread and the target thread are in the same
> group.

Large swaths of proc are governed by those checks.

In general in the kernel whenever you are accessing another process to
perform an operation (especially reading) it will probably use
"ptrace access mode" checks.

You can see this by with "git grep ptrace_may_access" on the kernel source.

Eric

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2020-12-17 18:47 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-15 17:01 [Bug 210655] ptrace.2: documentation is incorrect about access checking threads in same thread group Alejandro Colomar (man-pages)
2020-12-15 18:31 ` Ted Estes
2020-12-15 18:34   ` Alejandro Colomar (man-pages)
2020-12-15 22:48     ` Alejandro Colomar (man-pages)
2020-12-16  0:55       ` Linus Torvalds
2020-12-16  7:34         ` Oleg Nesterov
2020-12-15 23:07 ` Jann Horn
2020-12-15 23:23   ` Alejandro Colomar (man-pages)
2020-12-15 23:25     ` Alejandro Colomar (man-pages)
2020-12-16  0:01       ` Jann Horn
2020-12-16  2:21         ` Ted Estes
2020-12-16  2:33           ` Jann Horn
2020-12-16  9:22             ` Alejandro Colomar (man-pages)
2020-12-17 18:45               ` Eric W. Biederman

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).