linux-kernel - Re: [PATCH v6 16/33] riscv/shstk: If needed allocate a new shadow stack on clone

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: <aa75cbd142c51b996423f18769d8b8d7ecc39081.camel@intel.com>
Date: Tue, 8 Oct 2024 22:55:29 +0000
From: "Edgecombe, Rick P" <rick.p.edgecombe@...el.com>
To: "corbet@....net" <corbet@....net>, "robh@...nel.org" <robh@...nel.org>,
	"lorenzo.stoakes@...cle.com" <lorenzo.stoakes@...cle.com>,
	"dave.hansen@...ux.intel.com" <dave.hansen@...ux.intel.com>,
	"debug@...osinc.com" <debug@...osinc.com>, "vbabka@...e.cz" <vbabka@...e.cz>,
	"brauner@...nel.org" <brauner@...nel.org>, "akpm@...ux-foundation.org"
	<akpm@...ux-foundation.org>, "palmer@...belt.com" <palmer@...belt.com>,
	"mingo@...hat.com" <mingo@...hat.com>, "paul.walmsley@...ive.com"
	<paul.walmsley@...ive.com>, "Liam.Howlett@...cle.com"
	<Liam.Howlett@...cle.com>, "tglx@...utronix.de" <tglx@...utronix.de>,
	"aou@...s.berkeley.edu" <aou@...s.berkeley.edu>, "oleg@...hat.com"
	<oleg@...hat.com>, "krzk+dt@...nel.org" <krzk+dt@...nel.org>,
	"conor@...nel.org" <conor@...nel.org>, "ebiederm@...ssion.com"
	<ebiederm@...ssion.com>, "hpa@...or.com" <hpa@...or.com>,
	"peterz@...radead.org" <peterz@...radead.org>, "arnd@...db.de"
	<arnd@...db.de>, "bp@...en8.de" <bp@...en8.de>, "kees@...nel.org"
	<kees@...nel.org>, "x86@...nel.org" <x86@...nel.org>, "shuah@...nel.org"
	<shuah@...nel.org>
CC: "broonie@...nel.org" <broonie@...nel.org>, "jim.shu@...ive.com"
	<jim.shu@...ive.com>, "alistair.francis@....com" <alistair.francis@....com>,
	"cleger@...osinc.com" <cleger@...osinc.com>, "kito.cheng@...ive.com"
	<kito.cheng@...ive.com>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>, "samitolvanen@...gle.com"
	<samitolvanen@...gle.com>, "evan@...osinc.com" <evan@...osinc.com>,
	"linux-mm@...ck.org" <linux-mm@...ck.org>, "linux-arch@...r.kernel.org"
	<linux-arch@...r.kernel.org>, "atishp@...osinc.com" <atishp@...osinc.com>,
	"andybnac@...il.com" <andybnac@...il.com>, "devicetree@...r.kernel.org"
	<devicetree@...r.kernel.org>, "charlie@...osinc.com" <charlie@...osinc.com>,
	"linux-doc@...r.kernel.org" <linux-doc@...r.kernel.org>,
	"linux-fsdevel@...r.kernel.org" <linux-fsdevel@...r.kernel.org>,
	"linux-kselftest@...r.kernel.org" <linux-kselftest@...r.kernel.org>,
	"richard.henderson@...aro.org" <richard.henderson@...aro.org>,
	"linux-riscv@...ts.infradead.org" <linux-riscv@...ts.infradead.org>,
	"alexghiti@...osinc.com" <alexghiti@...osinc.com>
Subject: Re: [PATCH v6 16/33] riscv/shstk: If needed allocate a new shadow
 stack on clone

On Tue, 2024-10-08 at 15:36 -0700, Deepak Gupta wrote:
> +unsigned long shstk_alloc_thread_stack(struct task_struct *tsk,
> +					   const struct kernel_clone_args *args)
> +{
> +	unsigned long addr, size;
> +
> +	/* If shadow stack is not supported, return 0 */
> +	if (!cpu_supports_shadow_stack())
> +		return 0;
> +
> +	/*
> +	 * If shadow stack is not enabled on the new thread, skip any
> +	 * switch to a new shadow stack.
> +	 */
> +	if (!is_shstk_enabled(tsk))
> +		return 0;
> +
> +	/*
> +	 * For CLONE_VFORK the child will share the parents shadow stack.
> +	 * Set base = 0 and size = 0, this is special means to track this state
> +	 * so the freeing logic run for child knows to leave it alone.
> +	 */
> +	if (args->flags & CLONE_VFORK) {
> +		set_shstk_base(tsk, 0, 0);
> +		return 0;
> +	}
> +
> +	/*
> +	 * For !CLONE_VM the child will use a copy of the parents shadow
> +	 * stack.
> +	 */
> +	if (!(args->flags & CLONE_VM))
> +		return 0;
> +
> +	/*
> +	 * reaching here means, CLONE_VM was specified and thus a separate shadow
> +	 * stack is needed for new cloned thread. Note: below allocation is happening
> +	 * using current mm.
> +	 */
> +	size = calc_shstk_size(args->stack_size);
> +	addr = allocate_shadow_stack(0, size, 0, false);
> +	if (IS_ERR_VALUE(addr))
> +		return addr;
> +
> +	set_shstk_base(tsk, addr, size);
> +
> +	return addr + size;
> +}

A lot of this patch and the previous one is similar to x86's and arm's. It great
that we can have consistency around this behavior.

There might be enough consistency to refactor some of the arch code into a
kernel/shstk.c.

Should we try?