Re: [PATCH 2/4] tee: Remove vmalloc page support

From: Jens Wiklander
Date: Mon Oct 03 2022 - 02:41:53 EST


Hi Ira,

On Sun, Oct 2, 2022 at 2:23 AM <ira.weiny@xxxxxxxxx> wrote:
>
> From: Ira Weiny <ira.weiny@xxxxxxxxx>
>
> The kernel pages used by shm_get_kernel_pages() are allocated using
> GFP_KERNEL through the following call stack:
>
> trusted_instantiate()
> trusted_payload_alloc() -> GFP_KERNEL
> <trusted key op>
> tee_shm_register_kernel_buf()
> register_shm_helper()
> shm_get_kernel_pages()
>
> Where <trusted key op> is one of:
>
> trusted_key_unseal()
> trusted_key_get_random()
> trusted_key_seal()
>
> Remove the vmalloc page support from shm_get_kernel_pages(). Replace
> with a warn on once.
>
> Cc: Jens Wiklander <jens.wiklander@xxxxxxxxxx>
> Cc: Al Viro <viro@xxxxxxxxxxxxxxxxxx>
> Cc: "Fabio M. De Francesco" <fmdefrancesco@xxxxxxxxx>
> Cc: Christoph Hellwig <hch@xxxxxx>
> Cc: Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx>
> Signed-off-by: Ira Weiny <ira.weiny@xxxxxxxxx>
>
> ---
> Jens I went with the suggestion from Linus and Christoph and rejected
> vmalloc addresses. I did not hear back from you regarding Linus'
> question if the vmalloc page support was required by an up coming patch
> set or not. So I assumed it was something out of tree.

Yes, that's correctly assumed, sorry for not confirming that earlier.

Reviewed-by: Jens Wiklander <jens.wiklander@xxxxxxxxxx>

Thanks,
Jens

> ---
> drivers/tee/tee_shm.c | 36 ++++++++++++------------------------
> 1 file changed, 12 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/tee/tee_shm.c b/drivers/tee/tee_shm.c
> index 27295bda3e0b..527a6eabc03e 100644
> --- a/drivers/tee/tee_shm.c
> +++ b/drivers/tee/tee_shm.c
> @@ -24,37 +24,25 @@ static void shm_put_kernel_pages(struct page **pages, size_t page_count)
> static int shm_get_kernel_pages(unsigned long start, size_t page_count,
> struct page **pages)
> {
> + struct kvec *kiov;
> size_t n;
> int rc;
>
> - if (is_vmalloc_addr((void *)start)) {
> - struct page *page;
> -
> - for (n = 0; n < page_count; n++) {
> - page = vmalloc_to_page((void *)(start + PAGE_SIZE * n));
> - if (!page)
> - return -ENOMEM;
> -
> - get_page(page);
> - pages[n] = page;
> - }
> - rc = page_count;
> - } else {
> - struct kvec *kiov;
> -
> - kiov = kcalloc(page_count, sizeof(*kiov), GFP_KERNEL);
> - if (!kiov)
> - return -ENOMEM;
> + if (WARN_ON_ONCE(is_vmalloc_addr((void *)start)))
> + return -EINVAL;
>
> - for (n = 0; n < page_count; n++) {
> - kiov[n].iov_base = (void *)(start + n * PAGE_SIZE);
> - kiov[n].iov_len = PAGE_SIZE;
> - }
> + kiov = kcalloc(page_count, sizeof(*kiov), GFP_KERNEL);
> + if (!kiov)
> + return -ENOMEM;
>
> - rc = get_kernel_pages(kiov, page_count, 0, pages);
> - kfree(kiov);
> + for (n = 0; n < page_count; n++) {
> + kiov[n].iov_base = (void *)(start + n * PAGE_SIZE);
> + kiov[n].iov_len = PAGE_SIZE;
> }
>
> + rc = get_kernel_pages(kiov, page_count, 0, pages);
> + kfree(kiov);
> +
> return rc;
> }
>
> --
> 2.37.2
>