diff mbox series

[man-pages,v3,3/4] process_madvise.2: fix capability and ptrace requirements

Message ID 20221021223300.3675201-4-zokeefe@google.com (mailing list archive)
State New
Headers show
Series Add MADV_COLLAPSE documentation | expand

Commit Message

Zach O'Keefe Oct. 21, 2022, 10:32 p.m. UTC 
From: Zach O'Keefe <zokeefe@google.com>

The initial commit of process_madvise(2) to Linux, commit ecb8ac8b1f14
("mm/madvise: introduce process_madvise() syscall: an external memory
hinting API"), relied on PTRACE_MODE_ATTACH_FSCREDS (see ptrace(2)),
but was amended by commit 96cfe2c0fd23 ("mm/madvise: replace ptrace
attach requirement for process_madvise") which replaced this with a
combination of PTRACE_MODE_READ and CAP_SYS_NICE (PTRACE_MODE_READ to
prevent leaking ASLR metadata and CAP_SYS_NICE for influencing process
performance).

The initial commit of process_madvise(2) to man-pages project, made
after the second patch, included two errors:

1) CAP_SYS_ADMIN instead of CAP_SYS_NICE
2) PTRACE_MODE_READ_REALCREDS instead of PTRACE_MODE_READ_FSCREDS

Correct this in the man-page for process_madvise(2).

Fixes: a144f458b ("process_madvise.2: Document process_madvise(2)")
Cc: Suren Baghdasaryan <surenb@google.com>
Cc: Minchan Kim <minchan@kernel.org>
Signed-off-by: Zach O'Keefe <zokeefe@google.com>
---
 man2/process_madvise.2 | 21 +++++++++++++++++----
 1 file changed, 17 insertions(+), 4 deletions(-)

Comments

Alejandro Colomar Oct. 30, 2022, 11:50 a.m. UTC | #1 
Hi Suren,

On 10/22/22 00:32, Zach OKeefe wrote:
> From: Zach O'Keefe <zokeefe@google.com>
> 
> The initial commit of process_madvise(2) to Linux, commit ecb8ac8b1f14
> ("mm/madvise: introduce process_madvise() syscall: an external memory
> hinting API"), relied on PTRACE_MODE_ATTACH_FSCREDS (see ptrace(2)),
> but was amended by commit 96cfe2c0fd23 ("mm/madvise: replace ptrace
> attach requirement for process_madvise") which replaced this with a
> combination of PTRACE_MODE_READ and CAP_SYS_NICE (PTRACE_MODE_READ to
> prevent leaking ASLR metadata and CAP_SYS_NICE for influencing process
> performance).
> 
> The initial commit of process_madvise(2) to man-pages project, made
> after the second patch, included two errors:
> 
> 1) CAP_SYS_ADMIN instead of CAP_SYS_NICE
> 2) PTRACE_MODE_READ_REALCREDS instead of PTRACE_MODE_READ_FSCREDS
> 
> Correct this in the man-page for process_madvise(2).
> 
> Fixes: a144f458b ("process_madvise.2: Document process_madvise(2)")
> Cc: Suren Baghdasaryan <surenb@google.com>
> Cc: Minchan Kim <minchan@kernel.org>
> Signed-off-by: Zach O'Keefe <zokeefe@google.com>

You added your Reviewed-by tag to v2 of this patch.  I guess you'd like to put 
it in this one too, but since it changed slightly, I'd like you to confirm.

Thanks,

Alex

> ---
>   man2/process_madvise.2 | 21 +++++++++++++++++----
>   1 file changed, 17 insertions(+), 4 deletions(-)
> 
> diff --git a/man2/process_madvise.2 b/man2/process_madvise.2
> index 6208206e4..44d3b94e8 100644
> --- a/man2/process_madvise.2
> +++ b/man2/process_madvise.2
> @@ -105,16 +105,20 @@ remote process.
>   No further elements will be processed beyond that point.
>   (See the discussion regarding partial advice in RETURN VALUE.)
>   .PP
> -Permission to apply advice to another process is governed by a
> +.\" commit 96cfe2c0fd23ea7c2368d14f769d287e7ae1082e
> +Starting in Linux 5.12,
> +permission to apply advice to another process is governed by
>   ptrace access mode
> -.B PTRACE_MODE_READ_REALCREDS
> +.B PTRACE_MODE_READ_FSCREDS
>   check (see
>   .BR ptrace (2));
>   in addition,
>   because of the performance implications of applying the advice,
>   the caller must have the
> -.B CAP_SYS_ADMIN
> -capability.
> +.B CAP_SYS_NICE
> +capability
> +(see
> +.BR capabilities (7)).
>   .SH RETURN VALUE
>   On success,
>   .BR process_madvise ()
> @@ -180,6 +184,15 @@ configuration option.
>   The
>   .BR process_madvise ()
>   system call is Linux-specific.
> +.SH NOTES
> +When this system call first appeared in Linux 5.10,
> +permission to apply advice to another process was entirely governed by
> +ptrace access mode
> +.B PTRACE_MODE_ATTACH_FSCREDS
> +check (see
> +.BR ptrace (2)).
> +This requirement was relaxed in Linux 5.12 so that the caller didn't require
> +full control over the target process.
>   .SH SEE ALSO
>   .BR madvise (2),
>   .BR pidfd_open (2),
Suren Baghdasaryan Oct. 31, 2022, 7:13 p.m. UTC | #2 
Hi Alex,

On Sun, Oct 30, 2022 at 4:50 AM Alejandro Colomar
<alx.manpages@gmail.com> wrote:
>
> Hi Suren,
>
> On 10/22/22 00:32, Zach OKeefe wrote:
> > From: Zach O'Keefe <zokeefe@google.com>
> >
> > The initial commit of process_madvise(2) to Linux, commit ecb8ac8b1f14
> > ("mm/madvise: introduce process_madvise() syscall: an external memory
> > hinting API"), relied on PTRACE_MODE_ATTACH_FSCREDS (see ptrace(2)),
> > but was amended by commit 96cfe2c0fd23 ("mm/madvise: replace ptrace
> > attach requirement for process_madvise") which replaced this with a
> > combination of PTRACE_MODE_READ and CAP_SYS_NICE (PTRACE_MODE_READ to
> > prevent leaking ASLR metadata and CAP_SYS_NICE for influencing process
> > performance).
> >
> > The initial commit of process_madvise(2) to man-pages project, made
> > after the second patch, included two errors:
> >
> > 1) CAP_SYS_ADMIN instead of CAP_SYS_NICE
> > 2) PTRACE_MODE_READ_REALCREDS instead of PTRACE_MODE_READ_FSCREDS
> >
> > Correct this in the man-page for process_madvise(2).
> >
> > Fixes: a144f458b ("process_madvise.2: Document process_madvise(2)")
> > Cc: Suren Baghdasaryan <surenb@google.com>
> > Cc: Minchan Kim <minchan@kernel.org>
> > Signed-off-by: Zach O'Keefe <zokeefe@google.com>

Reviewed-by: Suren Baghdasaryan <surenb@google.com>

>
> You added your Reviewed-by tag to v2 of this patch.  I guess you'd like to put
> it in this one too, but since it changed slightly, I'd like you to confirm.

Thanks for the reminder!
Suren.

>
> Thanks,
>
> Alex
>
> > ---
> >   man2/process_madvise.2 | 21 +++++++++++++++++----
> >   1 file changed, 17 insertions(+), 4 deletions(-)
> >
> > diff --git a/man2/process_madvise.2 b/man2/process_madvise.2
> > index 6208206e4..44d3b94e8 100644
> > --- a/man2/process_madvise.2
> > +++ b/man2/process_madvise.2
> > @@ -105,16 +105,20 @@ remote process.
> >   No further elements will be processed beyond that point.
> >   (See the discussion regarding partial advice in RETURN VALUE.)
> >   .PP
> > -Permission to apply advice to another process is governed by a
> > +.\" commit 96cfe2c0fd23ea7c2368d14f769d287e7ae1082e
> > +Starting in Linux 5.12,
> > +permission to apply advice to another process is governed by
> >   ptrace access mode
> > -.B PTRACE_MODE_READ_REALCREDS
> > +.B PTRACE_MODE_READ_FSCREDS
> >   check (see
> >   .BR ptrace (2));
> >   in addition,
> >   because of the performance implications of applying the advice,
> >   the caller must have the
> > -.B CAP_SYS_ADMIN
> > -capability.
> > +.B CAP_SYS_NICE
> > +capability
> > +(see
> > +.BR capabilities (7)).
> >   .SH RETURN VALUE
> >   On success,
> >   .BR process_madvise ()
> > @@ -180,6 +184,15 @@ configuration option.
> >   The
> >   .BR process_madvise ()
> >   system call is Linux-specific.
> > +.SH NOTES
> > +When this system call first appeared in Linux 5.10,
> > +permission to apply advice to another process was entirely governed by
> > +ptrace access mode
> > +.B PTRACE_MODE_ATTACH_FSCREDS
> > +check (see
> > +.BR ptrace (2)).
> > +This requirement was relaxed in Linux 5.12 so that the caller didn't require
> > +full control over the target process.
> >   .SH SEE ALSO
> >   .BR madvise (2),
> >   .BR pidfd_open (2),
>
> --
> <http://www.alejandro-colomar.es/>
Alejandro Colomar Oct. 31, 2022, 8:24 p.m. UTC | #3 
Hi Suren, Zach,

On 10/31/22 20:13, Suren Baghdasaryan wrote:
> Hi Alex,
> 
> On Sun, Oct 30, 2022 at 4:50 AM Alejandro Colomar
> <alx.manpages@gmail.com> wrote:
>>
>> Hi Suren,
>>
>> On 10/22/22 00:32, Zach OKeefe wrote:
>>> From: Zach O'Keefe <zokeefe@google.com>
>>>
>>> The initial commit of process_madvise(2) to Linux, commit ecb8ac8b1f14
>>> ("mm/madvise: introduce process_madvise() syscall: an external memory
>>> hinting API"), relied on PTRACE_MODE_ATTACH_FSCREDS (see ptrace(2)),
>>> but was amended by commit 96cfe2c0fd23 ("mm/madvise: replace ptrace
>>> attach requirement for process_madvise") which replaced this with a
>>> combination of PTRACE_MODE_READ and CAP_SYS_NICE (PTRACE_MODE_READ to
>>> prevent leaking ASLR metadata and CAP_SYS_NICE for influencing process
>>> performance).
>>>
>>> The initial commit of process_madvise(2) to man-pages project, made
>>> after the second patch, included two errors:
>>>
>>> 1) CAP_SYS_ADMIN instead of CAP_SYS_NICE
>>> 2) PTRACE_MODE_READ_REALCREDS instead of PTRACE_MODE_READ_FSCREDS
>>>
>>> Correct this in the man-page for process_madvise(2).
>>>
>>> Fixes: a144f458b ("process_madvise.2: Document process_madvise(2)")
>>> Cc: Suren Baghdasaryan <surenb@google.com>
>>> Cc: Minchan Kim <minchan@kernel.org>
>>> Signed-off-by: Zach O'Keefe <zokeefe@google.com>
> 
> Reviewed-by: Suren Baghdasaryan <surenb@google.com>

Thanks!  Patch applied.

> 
>>
>> You added your Reviewed-by tag to v2 of this patch.  I guess you'd like to put
>> it in this one too, but since it changed slightly, I'd like you to confirm.
> 
> Thanks for the reminder!

:)

Cheers,

Alex

> Suren.
> 
>>
>> Thanks,
>>
>> Alex
>>
>>> ---
>>>    man2/process_madvise.2 | 21 +++++++++++++++++----
>>>    1 file changed, 17 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/man2/process_madvise.2 b/man2/process_madvise.2
>>> index 6208206e4..44d3b94e8 100644
>>> --- a/man2/process_madvise.2
>>> +++ b/man2/process_madvise.2
>>> @@ -105,16 +105,20 @@ remote process.
>>>    No further elements will be processed beyond that point.
>>>    (See the discussion regarding partial advice in RETURN VALUE.)
>>>    .PP
>>> -Permission to apply advice to another process is governed by a
>>> +.\" commit 96cfe2c0fd23ea7c2368d14f769d287e7ae1082e
>>> +Starting in Linux 5.12,
>>> +permission to apply advice to another process is governed by
>>>    ptrace access mode
>>> -.B PTRACE_MODE_READ_REALCREDS
>>> +.B PTRACE_MODE_READ_FSCREDS
>>>    check (see
>>>    .BR ptrace (2));
>>>    in addition,
>>>    because of the performance implications of applying the advice,
>>>    the caller must have the
>>> -.B CAP_SYS_ADMIN
>>> -capability.
>>> +.B CAP_SYS_NICE
>>> +capability
>>> +(see
>>> +.BR capabilities (7)).
>>>    .SH RETURN VALUE
>>>    On success,
>>>    .BR process_madvise ()
>>> @@ -180,6 +184,15 @@ configuration option.
>>>    The
>>>    .BR process_madvise ()
>>>    system call is Linux-specific.
>>> +.SH NOTES
>>> +When this system call first appeared in Linux 5.10,
>>> +permission to apply advice to another process was entirely governed by
>>> +ptrace access mode
>>> +.B PTRACE_MODE_ATTACH_FSCREDS
>>> +check (see
>>> +.BR ptrace (2)).
>>> +This requirement was relaxed in Linux 5.12 so that the caller didn't require
>>> +full control over the target process.
>>>    .SH SEE ALSO
>>>    .BR madvise (2),
>>>    .BR pidfd_open (2),
>>
>> --
>> <http://www.alejandro-colomar.es/>
Zach O'Keefe Oct. 31, 2022, 9:25 p.m. UTC | #4 
On Mon, Oct 31, 2022 at 1:24 PM Alejandro Colomar
<alx.manpages@gmail.com> wrote:
>
> Hi Suren, Zach,
>
> On 10/31/22 20:13, Suren Baghdasaryan wrote:
> > Hi Alex,
> >
> > On Sun, Oct 30, 2022 at 4:50 AM Alejandro Colomar
> > <alx.manpages@gmail.com> wrote:
> >>
> >> Hi Suren,
> >>
> >> On 10/22/22 00:32, Zach OKeefe wrote:
> >>> From: Zach O'Keefe <zokeefe@google.com>
> >>>
> >>> The initial commit of process_madvise(2) to Linux, commit ecb8ac8b1f14
> >>> ("mm/madvise: introduce process_madvise() syscall: an external memory
> >>> hinting API"), relied on PTRACE_MODE_ATTACH_FSCREDS (see ptrace(2)),
> >>> but was amended by commit 96cfe2c0fd23 ("mm/madvise: replace ptrace
> >>> attach requirement for process_madvise") which replaced this with a
> >>> combination of PTRACE_MODE_READ and CAP_SYS_NICE (PTRACE_MODE_READ to
> >>> prevent leaking ASLR metadata and CAP_SYS_NICE for influencing process
> >>> performance).
> >>>
> >>> The initial commit of process_madvise(2) to man-pages project, made
> >>> after the second patch, included two errors:
> >>>
> >>> 1) CAP_SYS_ADMIN instead of CAP_SYS_NICE
> >>> 2) PTRACE_MODE_READ_REALCREDS instead of PTRACE_MODE_READ_FSCREDS
> >>>
> >>> Correct this in the man-page for process_madvise(2).
> >>>
> >>> Fixes: a144f458b ("process_madvise.2: Document process_madvise(2)")
> >>> Cc: Suren Baghdasaryan <surenb@google.com>
> >>> Cc: Minchan Kim <minchan@kernel.org>
> >>> Signed-off-by: Zach O'Keefe <zokeefe@google.com>
> >
> > Reviewed-by: Suren Baghdasaryan <surenb@google.com>
>
> Thanks!  Patch applied.

Thanks Suren & Alex!

Best,
Zach

> >
> >>
> >> You added your Reviewed-by tag to v2 of this patch.  I guess you'd like to put
> >> it in this one too, but since it changed slightly, I'd like you to confirm.
> >
> > Thanks for the reminder!
>
> :)
>
> Cheers,
>
> Alex
>
> > Suren.
> >
> >>
> >> Thanks,
> >>
> >> Alex
> >>
> >>> ---
> >>>    man2/process_madvise.2 | 21 +++++++++++++++++----
> >>>    1 file changed, 17 insertions(+), 4 deletions(-)
> >>>
> >>> diff --git a/man2/process_madvise.2 b/man2/process_madvise.2
> >>> index 6208206e4..44d3b94e8 100644
> >>> --- a/man2/process_madvise.2
> >>> +++ b/man2/process_madvise.2
> >>> @@ -105,16 +105,20 @@ remote process.
> >>>    No further elements will be processed beyond that point.
> >>>    (See the discussion regarding partial advice in RETURN VALUE.)
> >>>    .PP
> >>> -Permission to apply advice to another process is governed by a
> >>> +.\" commit 96cfe2c0fd23ea7c2368d14f769d287e7ae1082e
> >>> +Starting in Linux 5.12,
> >>> +permission to apply advice to another process is governed by
> >>>    ptrace access mode
> >>> -.B PTRACE_MODE_READ_REALCREDS
> >>> +.B PTRACE_MODE_READ_FSCREDS
> >>>    check (see
> >>>    .BR ptrace (2));
> >>>    in addition,
> >>>    because of the performance implications of applying the advice,
> >>>    the caller must have the
> >>> -.B CAP_SYS_ADMIN
> >>> -capability.
> >>> +.B CAP_SYS_NICE
> >>> +capability
> >>> +(see
> >>> +.BR capabilities (7)).
> >>>    .SH RETURN VALUE
> >>>    On success,
> >>>    .BR process_madvise ()
> >>> @@ -180,6 +184,15 @@ configuration option.
> >>>    The
> >>>    .BR process_madvise ()
> >>>    system call is Linux-specific.
> >>> +.SH NOTES
> >>> +When this system call first appeared in Linux 5.10,
> >>> +permission to apply advice to another process was entirely governed by
> >>> +ptrace access mode
> >>> +.B PTRACE_MODE_ATTACH_FSCREDS
> >>> +check (see
> >>> +.BR ptrace (2)).
> >>> +This requirement was relaxed in Linux 5.12 so that the caller didn't require
> >>> +full control over the target process.
> >>>    .SH SEE ALSO
> >>>    .BR madvise (2),
> >>>    .BR pidfd_open (2),
> >>
> >> --
> >> <http://www.alejandro-colomar.es/>
>
> --
> <http://www.alejandro-colomar.es/>
diff mbox series

Patch

diff --git a/man2/process_madvise.2 b/man2/process_madvise.2
index 6208206e4..44d3b94e8 100644
--- a/man2/process_madvise.2
+++ b/man2/process_madvise.2
@@ -105,16 +105,20 @@  remote process.
 No further elements will be processed beyond that point.
 (See the discussion regarding partial advice in RETURN VALUE.)
 .PP
-Permission to apply advice to another process is governed by a
+.\" commit 96cfe2c0fd23ea7c2368d14f769d287e7ae1082e
+Starting in Linux 5.12,
+permission to apply advice to another process is governed by
 ptrace access mode
-.B PTRACE_MODE_READ_REALCREDS
+.B PTRACE_MODE_READ_FSCREDS
 check (see
 .BR ptrace (2));
 in addition,
 because of the performance implications of applying the advice,
 the caller must have the
-.B CAP_SYS_ADMIN
-capability.
+.B CAP_SYS_NICE
+capability
+(see
+.BR capabilities (7)).
 .SH RETURN VALUE
 On success,
 .BR process_madvise ()
@@ -180,6 +184,15 @@  configuration option.
 The
 .BR process_madvise ()
 system call is Linux-specific.
+.SH NOTES
+When this system call first appeared in Linux 5.10,
+permission to apply advice to another process was entirely governed by
+ptrace access mode
+.B PTRACE_MODE_ATTACH_FSCREDS
+check (see
+.BR ptrace (2)).
+This requirement was relaxed in Linux 5.12 so that the caller didn't require
+full control over the target process.
 .SH SEE ALSO
 .BR madvise (2),
 .BR pidfd_open (2),