Re: [PATCH v1 1/1] mm: fix race condition in the memory management

[Mike Rapoport <rppt@kernel.org>]

On Thu, Mar 12, 2026 at 01:14:38PM +0000, Hubert Mazur wrote:
> Subject: mm: fix race condition in the memory management

The prefix should be mm/execmem:
And this is not exactly a fix.

> When 'ARCH_HAS_EXECMEM_ROX' is being enabled the memory management
> system will use caching techniques to optimize the allocations. The
> logic tries to find the appropriate memory block based on requested
> size. This can fail if current allocations is not sufficient hence
> kernel allocates a new block large enough in regards to the request.
> After the allocation is done, the new block is being added to the
> free_areas tree and then - traverses the tree with hope to find the
> matching piecie of memory. The operations of allocating new memory and
> traversing the tree are not protected by mutex and thus there is a
> chance that some other process will "steal" this shiny new block. It's a

Does it actually happen in some environment or it's a theoretical issue?

> classic race condition for resources. Fix this accordingly by moving a
> new block of memory to busy fragments instead of free and return the
> pointer to memory. This simplifies the allocation logic since we don't
> firstly extend the free areas just to take it a bit later. In case the
> new memory allocation is required - do it and return to the caller.

It's hard to parse a single huge paragraph.
 
> Signed-off-by: Hubert Mazur <hmazur@google.com>
> ---
>  mm/execmem.c | 36 +++++++++++++++++-------------------
>  1 file changed, 17 insertions(+), 19 deletions(-)
> 
> diff --git a/mm/execmem.c b/mm/execmem.c
> index 810a4ba9c924..8aa44d19ec73 100644
> --- a/mm/execmem.c
> +++ b/mm/execmem.c
> @@ -307,32 +302,35 @@ static int execmem_cache_populate(struct execmem_range *range, size_t size)
>  	if (err)
>  		goto err_free_mem;
>  
> -	err = execmem_cache_add(p, alloc_size, GFP_KERNEL);
> +	/* Set new allocation as an already busy fragment */
> +	addr = (unsigned long)p;
> +	MA_STATE(mas, busy_areas, addr - 1, addr + 1);
> +	mas_set_range(&mas, addr, addr+size - 1);
> +
> +	mutex_lock(&execmem_cache.mutex);
> +	err = mas_store_gfp(&mas, (void *)addr, GFP_KERNEL);
> +	mutex_unlock(&execmem_cache.mutex);
> +
>  	if (err)
>  		goto err_reset_direct_map;
>  
> -	return 0;
> +	return p;

This is wrong. The caller asked for 'size' and got ALIGN(size, PMD_SIZE)
instead.

>  
>  err_reset_direct_map:
>  	execmem_set_direct_map_valid(vm, true);
>  err_free_mem:
>  	vfree(p);
> -	return err;
> +	return NULL;
>  }

-- 
Sincerely yours,
Mike.