linux-kernel - Re: [PATCH v20 4/8] fork: Add shadow stack support to clone3()

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <734e4c2c-a478-4019-86f7-4965c2b042e1@sirena.org.uk>
Date: Wed, 3 Sep 2025 11:01:05 +0100
From: Mark Brown <broonie@...nel.org>
To: Catalin Marinas <catalin.marinas@....com>
Cc: "Rick P. Edgecombe" <rick.p.edgecombe@...el.com>,
	Deepak Gupta <debug@...osinc.com>,
	Szabolcs Nagy <Szabolcs.Nagy@....com>,
	"H.J. Lu" <hjl.tools@...il.com>,
	Florian Weimer <fweimer@...hat.com>,
	Thomas Gleixner <tglx@...utronix.de>,
	Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
	Dave Hansen <dave.hansen@...ux.intel.com>, x86@...nel.org,
	"H. Peter Anvin" <hpa@...or.com>,
	Peter Zijlstra <peterz@...radead.org>,
	Juri Lelli <juri.lelli@...hat.com>,
	Vincent Guittot <vincent.guittot@...aro.org>,
	Dietmar Eggemann <dietmar.eggemann@....com>,
	Steven Rostedt <rostedt@...dmis.org>,
	Ben Segall <bsegall@...gle.com>, Mel Gorman <mgorman@...e.de>,
	Valentin Schneider <vschneid@...hat.com>,
	Christian Brauner <brauner@...nel.org>,
	Shuah Khan <shuah@...nel.org>, linux-kernel@...r.kernel.org,
	Will Deacon <will@...nel.org>, jannh@...gle.com,
	Andrew Morton <akpm@...ux-foundation.org>,
	Yury Khrustalev <yury.khrustalev@....com>,
	Wilco Dijkstra <wilco.dijkstra@....com>,
	linux-kselftest@...r.kernel.org, linux-api@...r.kernel.org,
	Kees Cook <kees@...nel.org>
Subject: Re: [PATCH v20 4/8] fork: Add shadow stack support to clone3()

On Tue, Sep 02, 2025 at 10:02:07PM +0100, Catalin Marinas wrote:
> On Tue, Sep 02, 2025 at 11:21:48AM +0100, Mark Brown wrote:

> > +	mm = get_task_mm(p);
> > +	if (!mm)
> > +		return -EFAULT;

> In theory, I don't think we need the get_task_mm() -> mmget() since
> copy_mm() early on already did this and the task can't disappear from
> underneath while we are creating it.

mmget() will only have been done in the CLONE_VM case, if we're in the
!CLONE_VM case we do a dup_mm() but that also returns with a reference.
I didn't know if people would be happier with the reference clearly
taken by the code using things or not, the general pattern is that
whenever we're doing anything with remote VMs we take a reference.

> > +	mmap_read_lock(mm);
> > +
> > +	addr = untagged_addr_remote(mm, args->shadow_stack_token);
> > +	page = get_user_page_vma_remote(mm, addr, FOLL_FORCE | FOLL_WRITE,
> > +					&vma);

> However, I wonder whether it makes sense to use the remote mm access
> here at all. Does this code ever run without CLONE_VM? If not, this is
> all done within the current mm context.

Yes, userspace can select if it wants CLONE_VM or not so we should
handle that case.  We discussed this on prior versions and we felt that
while we couldn't immediately see the use case for !CLONE_VM there
wasn't a good reason to restrict the creativity of userspace developers,
and given that you can specify the regular stack in these cases it seems
logical that you'd also be able to specify the shadow stack.

> I can see the x86 shstk_alloc_thread_stack() returns early if !CLONE_VM.
> Similarly on arm64. I think the behaviour is preserved with this series
> but I'm not entirely sure from the contextual diff (I need to apply the
> patches locally).

That is all for the case where the kernel allocates and manages the
shadow stack, it's the behaviour that this series allows userspace to
override.

Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)