Message-ID: <aJ2Pm2XzcM3H4aTN@kernel.org>
Date: Thu, 14 Aug 2025 10:26:19 +0300
From: Mike Rapoport <rppt@...nel.org>
To: Maciej Wieczor-Retman <maciej.wieczor-retman@...el.com>
Cc: nathan@...nel.org, arnd@...db.de, broonie@...nel.org,
	Liam.Howlett@...cle.com, urezki@...il.com, will@...nel.org,
	kaleshsingh@...gle.com, leitao@...ian.org, coxu@...hat.com,
	surenb@...gle.com, akpm@...ux-foundation.org, luto@...nel.org,
	jpoimboe@...nel.org, changyuanl@...gle.com, hpa@...or.com,
	dvyukov@...gle.com, kas@...nel.org, corbet@....net,
	vincenzo.frascino@....com, smostafa@...gle.com,
	nick.desaulniers+lkml@...il.com, morbo@...gle.com,
	andreyknvl@...il.com, alexander.shishkin@...ux.intel.com,
	thiago.bauermann@...aro.org, catalin.marinas@....com,
	ryabinin.a.a@...il.com, jan.kiszka@...mens.com, jbohac@...e.cz,
	dan.j.williams@...el.com, joel.granados@...nel.org,
	baohua@...nel.org, kevin.brodsky@....com, nicolas.schier@...ux.dev,
	pcc@...gle.com, andriy.shevchenko@...ux.intel.com,
	wei.liu@...nel.org, bp@...en8.de, ada.coupriediaz@....com,
	xin@...or.com, pankaj.gupta@....com, vbabka@...e.cz,
	glider@...gle.com, jgross@...e.com, kees@...nel.org,
	jhubbard@...dia.com, joey.gouly@....com, ardb@...nel.org,
	thuth@...hat.com, pasha.tatashin@...een.com,
	kristina.martsenko@....com, bigeasy@...utronix.de,
	lorenzo.stoakes@...cle.com, jason.andryuk@....com, david@...hat.com,
	graf@...zon.com, wangkefeng.wang@...wei.com, ziy@...dia.com,
	mark.rutland@....com, dave.hansen@...ux.intel.com,
	samuel.holland@...ive.com, kbingham@...nel.org,
	trintaeoitogc@...il.com, scott@...amperecomputing.com,
	justinstitt@...gle.com, kuan-ying.lee@...onical.com, maz@...nel.org,
	tglx@...utronix.de, samitolvanen@...gle.com, mhocko@...e.com,
	nunodasneves@...ux.microsoft.com, brgerst@...il.com,
	willy@...radead.org, ubizjak@...il.com, peterz@...radead.org,
	mingo@...hat.com, sohil.mehta@...el.com, linux-mm@...ck.org,
	linux-kbuild@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
	x86@...nel.org, llvm@...ts.linux.dev, kasan-dev@...glegroups.com,
	linux-doc@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v4 07/18] mm: x86: Untag addresses in EXECMEM_ROX related
 pointer arithmetic

On Tue, Aug 12, 2025 at 03:23:43PM +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() and within_range().
> 
> 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 v4:
> - Add patch to the series.
> 
>  arch/x86/mm/pat/set_memory.c | 1 +
>  mm/execmem.c                 | 4 +++-
>  mm/vmalloc.c                 | 4 ++--
>  3 files changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/x86/mm/pat/set_memory.c b/arch/x86/mm/pat/set_memory.c
> index 8834c76f91c9..1f14a1297db0 100644
> --- a/arch/x86/mm/pat/set_memory.c
> +++ b/arch/x86/mm/pat/set_memory.c
> @@ -222,6 +222,7 @@ static inline void cpa_inc_lp_preserved(int level) { }
>  static inline int
>  within(unsigned long addr, unsigned long start, unsigned long end)
>  {
> +	addr = (unsigned long)kasan_reset_tag((void *)addr);
>  	return addr >= start && addr < end;
>  }
>  
> diff --git a/mm/execmem.c b/mm/execmem.c
> index 0822305413ec..743fa4a8c069 100644
> --- a/mm/execmem.c
> +++ b/mm/execmem.c
> @@ -191,6 +191,8 @@ static int execmem_cache_add_locked(void *ptr, size_t size, gfp_t gfp_mask)
>  	unsigned long lower, upper;
>  	void *area = NULL;
>  
> +	addr = arch_kasan_reset_tag(addr);

Shouldn't this use kasan_reset_tag()?
And the calls below as well?

Also this can be done when addr is initialized 

> +
>  	lower = addr;
>  	upper = addr + size - 1;
>  
> @@ -216,7 +218,7 @@ static int execmem_cache_add(void *ptr, size_t size, gfp_t gfp_mask)
>  static bool within_range(struct execmem_range *range, struct ma_state *mas,
>  			 size_t size)
>  {
> -	unsigned long addr = mas->index;
> +	unsigned long addr = arch_kasan_reset_tag(mas->index);

AFAIU, we use plain address without the tag as an index in
execmem_cache_add(), so here mas->index will be a plain address as well
  
>  	if (addr >= range->start && addr + size < range->end)
>  		return true;
> diff --git a/mm/vmalloc.c b/mm/vmalloc.c
> index 6dbcdceecae1..83d666e4837a 100644
> --- a/mm/vmalloc.c
> +++ b/mm/vmalloc.c
> @@ -3328,8 +3328,8 @@ static void vm_reset_perms(struct vm_struct *area)
>  			unsigned long page_size;
>  
>  			page_size = PAGE_SIZE << page_order;
> -			start = min(addr, start);
> -			end = max(addr + page_size, end);
> +			start = min((unsigned long)arch_kasan_reset_tag(addr), start);
> +			end = max((unsigned long)arch_kasan_reset_tag(addr) + page_size, end);
>  			flush_dmap = 1;
>  		}
>  	}
> -- 
> 2.50.1
> 

-- 
Sincerely yours,
Mike.

Powered by blists - more mailing lists