Re: [PATCH man-pages v2 3/4] process_madvise.2: CAP_SYS_ADMIN cleanup

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

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

Hey Zach,

On 10/21/22 18:16, Zach O'Keefe wrote:
> Hey Alex!
> 
> Thanks for taking the time to review!
> 
> On Fri, Oct 21, 2022 at 5:41 AM Alejandro Colomar
> <alx.manpages@gmail.com> wrote:
>>
>> On 10/21/22 14:37, Alejandro Colomar wrote:
>>> On 10/19/22 01:50, Zach OKeefe wrote:
>>>> From: Zach O'Keefe <zokeefe@google.com>
>>>>
>>>> The initial commit of process_madvise(2) to man-pages project included
>>>> an error, indicating that CAP_SYS_ADMIN capability was required when, in
>>>> fact, CAP_SYS_NICE was the required capability.
>>>>
>>>> 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).
>>
>> [...]
>>
>>> If I understand the paragraph above, from 5.10 to 5.12 the capability required
>>> was CAP_SYS_ADMIN?
>>
>> Or was it CAP_SYS_PTRACE?
> 
> Starting in 5.10, there was no CAP_* capability requirement - only
> PTRACE_MODE_ATTACH_FSCREDS (aka PTRACE_MODE_ATTACH |
> PTRACE_MODE_REALCREDS). Now, my understanding of the algorithm
> employed for ptrace access mode checking isn't to be trusted, but
> AFAIK, a caller having CAP_SYS_PTRACE in the target's user namespace
> (directly or transitively) isn't required to pass this (though it
> makes it easier). ptrace(2) has an overview of the algorithm.
> 
> Starting in 5.12, CAP_SYS_NICE was added as a requirement, and the
> ptrace algorithm used changed to PTRACE_MODE_READ.

Understood.

> 
> If you think recording the differences in kernel versions in the
> man-page is important, let me know and I can amend this patch.

Yes; since it was live during 2 versions, I think we should at least mention it. 
  A couple of lines in NOTES might be enough.

Thanks,

Alex

> 
> Thanks,
> Zcah
> 
>> --
>> <http://www.alejandro-colomar.es/>

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

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