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

[Alejandro Colomar <alx.manpages@gmail.com>]

[-- Attachment #1.1: Type: text/plain, Size: 3007 bytes --]

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),

-- 
<http://www.alejandro-colomar.es/>

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]