linux-kernel - Re: [PATCH v5 07/19] mm: x86: Untag addresses in EXECMEM

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 for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <aLAmW-UV6hv9k1LT@kernel.org>
Date: Thu, 28 Aug 2025 12:50:19 +0300
From: Mike Rapoport <rppt@...nel.org>
To: Maciej Wieczor-Retman <maciej.wieczor-retman@...el.com>
Cc: sohil.mehta@...el.com, baohua@...nel.org, david@...hat.com,
	kbingham@...nel.org, weixugc@...gle.com, Liam.Howlett@...cle.com,
	alexandre.chartre@...cle.com, kas@...nel.org, mark.rutland@....com,
	trintaeoitogc@...il.com, axelrasmussen@...gle.com,
	yuanchu@...gle.com, joey.gouly@....com, samitolvanen@...gle.com,
	joel.granados@...nel.org, graf@...zon.com,
	vincenzo.frascino@....com, kees@...nel.org, ardb@...nel.org,
	thiago.bauermann@...aro.org, glider@...gle.com, thuth@...hat.com,
	kuan-ying.lee@...onical.com, pasha.tatashin@...een.com,
	nick.desaulniers+lkml@...il.com, vbabka@...e.cz,
	kaleshsingh@...gle.com, justinstitt@...gle.com,
	catalin.marinas@....com, alexander.shishkin@...ux.intel.com,
	samuel.holland@...ive.com, dave.hansen@...ux.intel.com,
	corbet@....net, xin@...or.com, dvyukov@...gle.com,
	tglx@...utronix.de, scott@...amperecomputing.com,
	jason.andryuk@....com, morbo@...gle.com, nathan@...nel.org,
	lorenzo.stoakes@...cle.com, mingo@...hat.com, brgerst@...il.com,
	kristina.martsenko@....com, bigeasy@...utronix.de, luto@...nel.org,
	jgross@...e.com, jpoimboe@...nel.org, urezki@...il.com,
	mhocko@...e.com, ada.coupriediaz@....com, hpa@...or.com,
	leitao@...ian.org, peterz@...radead.org, wangkefeng.wang@...wei.com,
	surenb@...gle.com, ziy@...dia.com, smostafa@...gle.com,
	ryabinin.a.a@...il.com, ubizjak@...il.com, jbohac@...e.cz,
	broonie@...nel.org, akpm@...ux-foundation.org,
	guoweikang.kernel@...il.com, pcc@...gle.com, jan.kiszka@...mens.com,
	nicolas.schier@...ux.dev, will@...nel.org, andreyknvl@...il.com,
	jhubbard@...dia.com, bp@...en8.de, x86@...nel.org,
	linux-doc@...r.kernel.org, linux-mm@...ck.org, llvm@...ts.linux.dev,
	linux-kbuild@...r.kernel.org, kasan-dev@...glegroups.com,
	linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH v5 07/19] mm: x86: Untag addresses in EXECMEM_ROX related
 pointer arithmetic

On Mon, Aug 25, 2025 at 10:24:32PM +0200, Maciej Wieczor-Retman wrote:
> ARCH_HAS_EXECMEM_ROX was re-enabled in x86 at Linux 6.14 release.
> Related code has multiple spots where page virtual addresses end up used
> as arguments in arithmetic operations. Combined with enabled tag-based
> KASAN it can result in pointers that don't point where they should or
> logical operations not giving expected results.
> 
> vm_reset_perms() calculates range's start and end addresses using min()
> and max() functions. To do that it compares pointers but some are not
> tagged - addr variable is, start and end variables aren't.
> 
> within() and within_range() can receive tagged addresses which get
> compared to untagged start and end variables.
> 
> Reset tags in addresses used as function arguments in min(), max(),
> within().
> 
> execmem_cache_add() adds tagged pointers to a maple tree structure,
> which then are incorrectly compared when walking the tree. That results
> in different pointers being returned later and page permission violation
> errors panicking the kernel.
> 
> Reset tag of the address range inserted into the maple tree inside
> execmem_cache_add().
> 
> Signed-off-by: Maciej Wieczor-Retman <maciej.wieczor-retman@...el.com>
> ---
> Changelog v5:
> - Remove the within_range() change.
> - arch_kasan_reset_tag -> kasan_reset_tag.
> 
> Changelog v4:
> - Add patch to the series.
> 
>  mm/execmem.c | 2 +-
>  mm/vmalloc.c | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/mm/execmem.c b/mm/execmem.c
> index 0822305413ec..f7b7bdacaec5 100644
> --- a/mm/execmem.c
> +++ b/mm/execmem.c
> @@ -186,7 +186,7 @@ static DECLARE_WORK(execmem_cache_clean_work, execmem_cache_clean);
>  static int execmem_cache_add_locked(void *ptr, size_t size, gfp_t gfp_mask)
>  {
>  	struct maple_tree *free_areas = &execmem_cache.free_areas;
> -	unsigned long addr = (unsigned long)ptr;
> +	unsigned long addr = (unsigned long)kasan_reset_tag(ptr);

Thinking more about it, we anyway reset tag in execmem_alloc() and return
untagged pointer to the caller. Let's just move kasan_reset_tag() to
execmem_vmalloc() so that we always use untagged pointers. Seems more
robust to me.

>  	MA_STATE(mas, free_areas, addr - 1, addr + 1);
>  	unsigned long lower, upper;
>  	void *area = NULL;
> diff --git a/mm/vmalloc.c b/mm/vmalloc.c
> index 6dbcdceecae1..c93893fb8dd4 100644
> --- a/mm/vmalloc.c
> +++ b/mm/vmalloc.c
> @@ -3322,7 +3322,7 @@ static void vm_reset_perms(struct vm_struct *area)
>  	 * the vm_unmap_aliases() flush includes the direct map.
>  	 */
>  	for (i = 0; i < area->nr_pages; i += 1U << page_order) {
> -		unsigned long addr = (unsigned long)page_address(area->pages[i]);
> +		unsigned long addr = (unsigned long)kasan_reset_tag(page_address(area->pages[i]));

This is not strictly related to execemem, there may other users of
VM_FLUSH_RESET_PERMS.

Regardless, I wonder how this works on arm64 with tags enabled?

Also, it's not the only place in the kernel that does (unsigned
long)page_address(page). Do other sites need to reset the tag as well?

>  
>  		if (addr) {
>  			unsigned long page_size;
> -- 
> 2.50.1
> 

-- 
Sincerely yours,
Mike.