Re: [DISCUSSION] proposed mctl() API

On Tue, Jun 10, 2025 at 04:03:07PM +0100, Usama Arif wrote:

On 30/05/2025 14:10, Lorenzo Stoakes wrote:

quoted

On Thu, May 29, 2025 at 06:21:55PM +0100, Usama Arif wrote:

quoted

My knowledge is security is limited, so please bare with me, but I actually
didn't understand the security issue and the need for CAP_SYS_ADMIN for
doing VM_(NO)HUGEPAGE.

A process can already madvise its own VMAs, and this is just doing that
for the entire process. And VM_INIT_DEF_MASK is already set to VM_NOHUGEPAGE
so it will be inherited by the parent. Just adding VM_HUGEPAGE shouldnt be
a issue? Inheriting MMF_VM_HUGEPAGE will mean that khugepaged would enter
for that process as well, which again doesnt seem like a security issue
to me.

W.R.T. the current process, the Issue is one Jann raised, in relation to
propagation of behaviour to privileged (e.g. setuid) processes.

But what is the actual security issue of having hugepages (or not having them) when
the process is running with setuid?

Speak to Jann about this. Security isn't my area. He gave feedback on this,
which is why I raised it, if you search through previous threads you can find
it.

I know the cgroup proposal has been shot down, but lets imagine if this was a cgroup
setting, similar to the other memory controls we have, for e.g. memory.swap.{max,high,peak}.

We can chown the cgroup so that the property is set by unprivileged process.

Having the process swap with setuid when the unprivileged process has swap disabled
in the cgroup is not the right behaviour. What currently happens is that the process
after obtaining the higher privilege level doesn't swap as well.

Similarly for hugepages, if it was a cgroup level setting, having the process give
hugepages always with setuid when the unprivileged user had it disabled it or vice versa
would not be the right behaviour.

Another example is PR_SET_MEMORY_MERGE, setuid does not change how it works as far as
I can tell.

So madlibs I dont see what the security issue is and why we would need to elevate privileges
to do this.

quoted

W.R.T. remote processes, obviously we want to make sure we are permitted to do
so.

I know that this needs to be future proof. But I don't actually know of a real world
usecase where we want to do any of these things for remote processes.
Whether its the existing per process changes like PR_SET_MEMORY_MERGE for KSM and
PR_SET_THP_DISABLE for THP or the newer proposals of PR_DEFAULT_MADV_(NO)HUGEPAGE
or Barrys proposal.
All of them are for the process itself (and its children by fork+exec) and not for
remote processes. As we try to make our changes usecase driven, I think we should
not add support for remote processes (which is another reason why I think this might
sit better in prctl).

I'm extremely confused as to why you think this propoal is predicated upon
remote process manipulation? It was simply suggested as a possibility for
increased flexibility.

We can just remove this parameter no?

It is entirely orthogonal to the prctl() stuff.

Overall at this point I share Matthew's point of view on this - we shouldn't be
doing any of this upstream.