[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <fcaa7ce6-3f03-4e3d-aa9f-1b1b53ed88f5@lucifer.local>
Date: Tue, 10 Jun 2025 16:17:33 +0100
From: Lorenzo Stoakes <lorenzo.stoakes@...cle.com>
To: Usama Arif <usamaarif642@...il.com>
Cc: Andrew Morton <akpm@...ux-foundation.org>,
Shakeel Butt <shakeel.butt@...ux.dev>,
"Liam R . Howlett" <Liam.Howlett@...cle.com>,
David Hildenbrand <david@...hat.com>, Vlastimil Babka <vbabka@...e.cz>,
Jann Horn <jannh@...gle.com>, Arnd Bergmann <arnd@...db.de>,
Christian Brauner <brauner@...nel.org>, SeongJae Park <sj@...nel.org>,
Mike Rapoport <rppt@...nel.org>, Johannes Weiner <hannes@...xchg.org>,
Barry Song <21cnbao@...il.com>, linux-mm@...ck.org,
linux-arch@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-api@...r.kernel.org, Pedro Falcato <pfalcato@...e.de>
Subject: 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:
> > 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.
Powered by blists - more mailing lists