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@xxxxxxxxx>
> ---
> 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.
