On Tue, Jun 10, 2025 at 04:03:07PM +0100, Usama Arif wrote: > > > On 30/05/2025 14:10, Lorenzo Stoakes wrote: > > On Thu, May 29, 2025 at 06:21:55PM +0100, Usama Arif wrote: > >> > >> > >> 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. > > > 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.