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

[Suren Baghdasaryan <surenb@google.com>]

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