Re: [PATCH 1/3] efi: stmm: Fix incorrect buffer allocation method

[Ilias Apalodimas]

(++cc Sumit and Kojima-san on their updated emails)

On Fri, 15 Aug 2025 at 22:12, Jan Kiszka <jan.kiszka@xxxxxxxxxxx> wrote:
>
> From: Jan Kiszka <jan.kiszka@xxxxxxxxxxx>
>
> The communication buffer allocated by setup_mm_hdr is later on passed to
> tee_shm_register_kernel_buf. The latter expects those buffers to be
> contiguous pages, but setup_mm_hdr just uses kmalloc. That can cause
> various corruptions or BUGs, specifically since 9aec2fb0fd5e, though it
> was broken before as well.
>
> Fix this by using alloc_pages_exact instead of kmalloc.
>
> Fixes: c44b6be62e8d ("efi: Add tee-based EFI variable driver")
> Signed-off-by: Jan Kiszka <jan.kiszka@xxxxxxxxxxx>
> ---

[...]

>         const efi_guid_t mm_var_guid = EFI_MM_VARIABLE_GUID;
>         struct efi_mm_communicate_header *mm_hdr;
> @@ -173,9 +174,12 @@ static void *setup_mm_hdr(u8 **dptr, size_t payload_size, size_t func,
>                 return NULL;
>         }
>
> -       comm_buf = kzalloc(MM_COMMUNICATE_HEADER_SIZE +
> -                                  MM_VARIABLE_COMMUNICATE_SIZE + payload_size,
> -                          GFP_KERNEL);
> +       *nr_pages = roundup(MM_COMMUNICATE_HEADER_SIZE +
> +                           MM_VARIABLE_COMMUNICATE_SIZE + payload_size,
> +                           PAGE_SIZE) / PAGE_SIZE;
> +
> +       comm_buf = alloc_pages_exact(*nr_pages * PAGE_SIZE,
> +                                    GFP_KERNEL | __GFP_ZERO);

Rename nr_pages to something else and skip division, multiplying.
Unless there's a reason I am missing?
Also doesn't alloc_pages_exact() already rounds things up?

>         if (!comm_buf) {
>                 *ret = EFI_OUT_OF_RESOURCES;
>                 return NULL;
> @@ -205,13 +209,14 @@ static efi_status_t get_max_payload(size_t *size)
>         struct smm_variable_payload_size *var_payload = NULL;
>         size_t payload_size;
>         u8 *comm_buf = NULL;

[...]

Thanks
/Ilias