Re: [RFC PATCH v8 04/14] irqchip/gic-v3-its: Allocate shared tables using CoCo shared memory allocator

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] irqchip/gic-v3-its: buddy allocator corruption due to discarded shared_size
--

commit d897529703a71c914b0334abcb563ffb37979469
Author: Aneesh Kumar K.V (Arm) <aneesh.kumar@kernel.org>

irqchip/gic-v3-its: Allocate shared tables using CoCo shared memory allocator

This commit updates the ITS allocator to route page allocations through
alloc_cc_shared_pages_node(). It reconstructs common allocation metadata
from the original ITS order and the architecture granule size on release.

> diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
> index ac5507acb1a0c..4814161df02ed 100644
> --- a/drivers/irqchip/irq-gic-v3-its.c
> +++ b/drivers/irqchip/irq-gic-v3-its.c

[ ... ]

> @@ -214,25 +213,13 @@ static gfp_t gfp_flags_quirk;
>  static struct page *its_alloc_pages_node(int node, gfp_t gfp,
>  					 unsigned int order)
>  {
> -	struct page *page;
> -	int ret = 0;
> -
> -	page = alloc_pages_node(node, gfp | gfp_flags_quirk, order);
> -
> -	if (!page)
> -		return NULL;
> +	struct cc_shared_pages mem;
>  
> -	ret = set_memory_decrypted((unsigned long)page_address(page),
> -				   1 << order);
> -	/*
> -	 * If set_memory_decrypted() fails then we don't know what state the
> -	 * page is in, so we can't free it. Instead we leak it.
> -	 * set_memory_decrypted() will already have WARNed.
> -	 */
> -	if (ret)
> +	if (alloc_cc_shared_pages_node(node, gfp | gfp_flags_quirk,
> +				       PAGE_SIZE << order, &mem))
>  		return NULL;
>  
> -	return page;
> +	return mem.page;
>  }

[Severity: High]
Is it safe to discard mem.shared_size here? The API contract seems to require
callers to retain the true allocated size so it can be passed back when
freeing the pages.

>  static struct page *its_alloc_pages(gfp_t gfp, unsigned int order)
> @@ -242,13 +229,15 @@ static struct page *its_alloc_pages(gfp_t gfp, unsigned int order)
>  
>  static void its_free_pages(void *addr, unsigned int order)
>  {
> -	/*
> -	 * If the memory cannot be encrypted again then we must leak the pages.
> -	 * set_memory_encrypted() will already have WARNed.
> -	 */
> -	if (set_memory_encrypted((unsigned long)addr, 1 << order))
> +	struct cc_shared_layout layout;
> +	struct cc_shared_pages mem;
> +
> +	if (WARN_ON(cc_shared_calc_layout(PAGE_SIZE << order, &layout)))
>  		return;
> -	free_pages((unsigned long)addr, order);
> +
> +	mem.page = virt_to_page(addr);
> +	mem.shared_size = layout.shared_size;
> +	free_cc_shared_pages(&mem);
>  }

[Severity: High]
Does recalculating the size here cause a mismatch on non-CC platforms?

When memory encryption is disabled, alloc_cc_shared_pages_node() does not
round up the allocation to the granule size; it simply allocates the
requested order.

However, cc_shared_calc_layout() unconditionally rounds up to the
architectural granule size. If the granule size exceeds the requested
allocation size, free_cc_shared_pages() will call __free_pages() with a
higher order than was actually allocated. Can this corrupt the buddy
allocator by freeing unallocated pages?

>  static struct gen_pool *itt_pool;
> @@ -273,7 +262,8 @@ static void *itt_alloc_pool(int node, int size)
>  		if (!page)
>  			break;
>  
> -		gen_pool_add(itt_pool, (unsigned long)page_address(page), PAGE_SIZE, node);
> +		gen_pool_add(itt_pool, (unsigned long)page_address(page),
> +			     cc_shared_granule_size(), node);
>  	} while (!addr);
>  
>  	return (void *)addr;

[Severity: High]
Will adding a hardcoded granule size to the generic pool advertise unallocated
out-of-bounds memory to consumers?

On non-CC platforms, the underlying allocation size is just a single page,
but this adds the full cc_shared_granule_size() bytes to the pool. Could
this lead to pool consumers accessing memory beyond what was actually
allocated?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260924100529.1398790-1-aneesh.kumar@kernel.org?part=4