linux-kernel - Re: [PATCH v7 7/8] execmem: add support for cache of large ROX pages

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] [day] [month] [year] [list]
Message-ID: <16863478-2195-435e-a899-559df097bc59@arm.com>
Date: Thu, 27 Feb 2025 11:13:29 +0000
From: Ryan Roberts <ryan.roberts@....com>
To: Mike Rapoport <rppt@...nel.org>, Andrew Morton
 <akpm@...ux-foundation.org>, Luis Chamberlain <mcgrof@...nel.org>,
 Dev Jain <dev.jain@....com>
Cc: Andreas Larsson <andreas@...sler.com>, Andy Lutomirski <luto@...nel.org>,
 Ard Biesheuvel <ardb@...nel.org>, Arnd Bergmann <arnd@...db.de>,
 Borislav Petkov <bp@...en8.de>, Brian Cain <bcain@...cinc.com>,
 Catalin Marinas <catalin.marinas@....com>,
 Christoph Hellwig <hch@...radead.org>,
 Christophe Leroy <christophe.leroy@...roup.eu>,
 Dave Hansen <dave.hansen@...ux.intel.com>, Dinh Nguyen
 <dinguyen@...nel.org>, Geert Uytterhoeven <geert@...ux-m68k.org>,
 Guo Ren <guoren@...nel.org>, Helge Deller <deller@....de>,
 Huacai Chen <chenhuacai@...nel.org>, Ingo Molnar <mingo@...hat.com>,
 Johannes Berg <johannes@...solutions.net>,
 John Paul Adrian Glaubitz <glaubitz@...sik.fu-berlin.de>,
 Kent Overstreet <kent.overstreet@...ux.dev>,
 "Liam R. Howlett" <Liam.Howlett@...cle.com>,
 Mark Rutland <mark.rutland@....com>, Masami Hiramatsu <mhiramat@...nel.org>,
 Matt Turner <mattst88@...il.com>, Max Filippov <jcmvbkbc@...il.com>,
 Michael Ellerman <mpe@...erman.id.au>, Michal Simek <monstr@...str.eu>,
 Oleg Nesterov <oleg@...hat.com>, Palmer Dabbelt <palmer@...belt.com>,
 Peter Zijlstra <peterz@...radead.org>, Richard Weinberger <richard@....at>,
 Russell King <linux@...linux.org.uk>, Song Liu <song@...nel.org>,
 Stafford Horne <shorne@...il.com>, Steven Rostedt <rostedt@...dmis.org>,
 Suren Baghdasaryan <surenb@...gle.com>,
 Thomas Bogendoerfer <tsbogend@...ha.franken.de>,
 Thomas Gleixner <tglx@...utronix.de>, Uladzislau Rezki <urezki@...il.com>,
 Vineet Gupta <vgupta@...nel.org>, Will Deacon <will@...nel.org>,
 bpf@...r.kernel.org, linux-alpha@...r.kernel.org,
 linux-arch@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
 linux-csky@...r.kernel.org, linux-hexagon@...r.kernel.org,
 linux-kernel@...r.kernel.org, linux-m68k@...ts.linux-m68k.org,
 linux-mips@...r.kernel.org, linux-mm@...ck.org,
 linux-modules@...r.kernel.org, linux-openrisc@...r.kernel.org,
 linux-parisc@...r.kernel.org, linux-riscv@...ts.infradead.org,
 linux-sh@...r.kernel.org, linux-snps-arc@...ts.infradead.org,
 linux-trace-kernel@...r.kernel.org, linux-um@...ts.infradead.org,
 linuxppc-dev@...ts.ozlabs.org, loongarch@...ts.linux.dev,
 sparclinux@...r.kernel.org, x86@...nel.org
Subject: Re: [PATCH v7 7/8] execmem: add support for cache of large ROX pages

Hi Mike,

Drive by review comments below...


On 23/10/2024 17:27, Mike Rapoport wrote:
> From: "Mike Rapoport (Microsoft)" <rppt@...nel.org>
> 
> Using large pages to map text areas reduces iTLB pressure and improves
> performance.
> 
> Extend execmem_alloc() with an ability to use huge pages with ROX
> permissions as a cache for smaller allocations.
> 
> To populate the cache, a writable large page is allocated from vmalloc with
> VM_ALLOW_HUGE_VMAP, filled with invalid instructions and then remapped as
> ROX.
> 
> The direct map alias of that large page is exculded from the direct map.
> 
> Portions of that large page are handed out to execmem_alloc() callers
> without any changes to the permissions.
> 
> When the memory is freed with execmem_free() it is invalidated again so
> that it won't contain stale instructions.
> 
> An architecture has to implement execmem_fill_trapping_insns() callback
> and select ARCH_HAS_EXECMEM_ROX configuration option to be able to use
> the ROX cache.
> 
> The cache is enabled on per-range basis when an architecture sets
> EXECMEM_ROX_CACHE flag in definition of an execmem_range.
> 
> Signed-off-by: Mike Rapoport (Microsoft) <rppt@...nel.org>
> Reviewed-by: Luis Chamberlain <mcgrof@...nel.org>
> Tested-by: kdevops <kdevops@...ts.linux.dev>
> ---

[...]

> +
> +static int execmem_cache_populate(struct execmem_range *range, size_t size)
> +{
> +	unsigned long vm_flags = VM_ALLOW_HUGE_VMAP;
> +	unsigned long start, end;
> +	struct vm_struct *vm;
> +	size_t alloc_size;
> +	int err = -ENOMEM;
> +	void *p;
> +
> +	alloc_size = round_up(size, PMD_SIZE);
> +	p = execmem_vmalloc(range, alloc_size, PAGE_KERNEL, vm_flags);

Shouldn't this be passing PAGE_KERNEL_ROX? Otherwise I don't see how the
allocated memory is ROX? I don't see any call below where you change the permission.

Given the range has the pgprot in it, you could just drop passing the pgprot
explicitly here and have execmem_vmalloc() use range->pgprot directly?

Thanks,
Ryan

> +	if (!p)
> +		return err;
> +
> +	vm = find_vm_area(p);
> +	if (!vm)
> +		goto err_free_mem;
> +
> +	/* fill memory with instructions that will trap */
> +	execmem_fill_trapping_insns(p, alloc_size, /* writable = */ true);
> +
> +	start = (unsigned long)p;
> +	end = start + alloc_size;
> +
> +	vunmap_range(start, end);
> +
> +	err = execmem_set_direct_map_valid(vm, false);
> +	if (err)
> +		goto err_free_mem;
> +
> +	err = vmap_pages_range_noflush(start, end, range->pgprot, vm->pages,
> +				       PMD_SHIFT);
> +	if (err)
> +		goto err_free_mem;
> +
> +	err = execmem_cache_add(p, alloc_size);
> +	if (err)
> +		goto err_free_mem;
> +
> +	return 0;
> +
> +err_free_mem:
> +	vfree(p);
> +	return err;
> +}

[...]