On Tue, Oct 27, 2020 at 01:11:40PM +0100, Michael Kerrisk (man-pages) wrote:

Hello Nicolas, Cyrill, and others,

@Nicolas, your commit ebd6de6812387a changed the capability 
requirements for the prctl_set_mm_exe_file() operation from

    ns_capable(CAP_SYS_ADMIN)

to

    ns_capable(CAP_SYS_ADMIN) || ns_capable(CAP_CHECKPOINT_RESTORE).

That's fine I guess, but while looking at that change, I found
an anomaly.

The same prctl_set_mm_exe_file() functionality is also available
via the prctl() PR_SET_MM_EXE_FILE operation, which was added
by Cyrill's commit b32dfe377102ce668. However, there the 
prctl_set_mm_exe_file() operation is guarded by a check

    capable(CAP_SYS_RESOURCE).

There are two things I note:

* The capability requirements are different in the two cases.
* In one case the checks are with ns_capable(), while in the 
  other case the check is with capable().

In both cases, the inconsistencies predate Nicolas's patch,
and appear to have been introduced in Kirill Tkhai's commit
4d28df6152aa3ff.

I'm not sure what is right, but those inconsistencies seem
seem odd, and presumably unintended. Similarly, I'm not
sure what fix, if any, should be applied. However, I thought
it worth mentioning these details, since the situation is odd
and surprising.

Hi Michael! This is more likely due to historical reasons:
the initial version of prctl(PR_SET_MM, ...) been operating
with individual fields and this was very unsafe. Because of
this we left it under CAP_SYS_RESOURCE (because you must have
enough rights to change such deep fields). Later we switched
to PR_SET_MM_MAP which is a safe version and allows to modify
memory map as a "whole" so we can do a precise check. And this
allowed us to relax requirements.

As to me the old PR_SET_MM should be deprecated and finally
removed from the kernel, but since it is a part of API we
can't do such thing easily.

Same time current PR_SET_MM internally is rather an alias
for PR_SET_MM_MAP because we create a temporary map and
pass it to the verification procedure so it looks like
we can relax requirements here to match the PR_SET_MM_MAP
call. But need to think maybe I miss something obvious here.