Re: [RFT PATCH] tee: shm: Remove refcounting of kernel pages

[Sven Püschel]

Hi Sumit,

On 2/13/26 12:33 PM, Sumit Garg wrote:
> From: Sumit Garg <sumit.garg@xxxxxxxxxxxxxxxx>
>
> Earlier TEE subsystem assumed to refcount all the memory pages to be
> shared with TEE implementation to be refcounted. However, the slab
> allocations within the kernel don't allow refcounting kernel pages.
>
> It is rather better to trust the kernel clients to not free pages while
> being shared with TEE implementation. Hence, remove refcounting of kernel
> pages from register_shm_helper() API.
>
> Fixes: b9c0e49abfca ("mm: decline to manipulate the refcount on a slab page")
> Reported-by: Marco Felsch <m.felsch@xxxxxxxxxxxxxx>
> Reported-by: Sven Püschel <s.pueschel@xxxxxxxxxxxxxx>
> Suggested-by: Matthew Wilcox <willy@xxxxxxxxxxxxx>
> Signed-off-by: Sumit Garg <sumit.garg@xxxxxxxxxxxxxxxx>
> ---
>   drivers/tee/tee_shm.c | 29 +----------------------------
>   1 file changed, 1 insertion(+), 28 deletions(-)
>
> diff --git a/drivers/tee/tee_shm.c b/drivers/tee/tee_shm.c
> index 4a47de4bb2e5..54e2ba3afb25 100644
> --- a/drivers/tee/tee_shm.c
> +++ b/drivers/tee/tee_shm.c
> @@ -23,29 +23,11 @@ struct tee_shm_dma_mem {
>   	struct page *page;
>   };
>   
> -static void shm_put_kernel_pages(struct page **pages, size_t page_count)
> -{
> -	size_t n;
> -
> -	for (n = 0; n < page_count; n++)
> -		put_page(pages[n]);
> -}
> -
> -static void shm_get_kernel_pages(struct page **pages, size_t page_count)
> -{
> -	size_t n;
> -
> -	for (n = 0; n < page_count; n++)
> -		get_page(pages[n]);
> -}
> -
>   static void release_registered_pages(struct tee_shm *shm)
>   {
>   	if (shm->pages) {
>   		if (shm->flags & TEE_SHM_USER_MAPPED)
>   			unpin_user_pages(shm->pages, shm->num_pages);
> -		else
> -			shm_put_kernel_pages(shm->pages, shm->num_pages);
>   
>   		kfree(shm->pages);
>   	}
> @@ -477,13 +459,6 @@ register_shm_helper(struct tee_context *ctx, struct iov_iter *iter, u32 flags,
>   		goto err_put_shm_pages;
>   	}
>   
> -	/*
> -	 * iov_iter_extract_kvec_pages does not get reference on the pages,
> -	 * get a reference on them.
> -	 */
> -	if (iov_iter_is_kvec(iter))
> -		shm_get_kernel_pages(shm->pages, num_pages);
> -
>   	shm->offset = off;
>   	shm->size = len;
>   	shm->num_pages = num_pages;
> @@ -497,10 +472,8 @@ register_shm_helper(struct tee_context *ctx, struct iov_iter *iter, u32 flags,
>   
>   	return shm;
>   err_put_shm_pages:
> -	if (!iov_iter_is_kvec(iter))
> +	if (iter_is_uvec(iter))

I've replaced (due to compile failures noted by the kernel test robot) 
this with the compiler suggested iter_is_iovec . I haven't checked, if 
this is the correct fix.

With this patch applied (on the upstream commit 970296997869), my 
reported stacktrace/warning doesn't occur anymore. I didn't do any 
extensive testing, but creating and using a trusted key with keyctl 
worked fine in my short test.

Sincerely
    Sven

>   		unpin_user_pages(shm->pages, shm->num_pages);
> -	else
> -		shm_put_kernel_pages(shm->pages, shm->num_pages);
>   err_free_shm_pages:
>   	kfree(shm->pages);
>   err_free_shm: