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

[Sumit Garg <sumit.garg@kernel.org>]

Hi Jan,

On Wed, Aug 20, 2025 at 05:05:43PM +0200, Jan Kiszka wrote:
> On 20.08.25 09:29, Ilias Apalodimas wrote:
> > (++cc Sumit and Kojima-san on their updated emails)
> > 
> > On Fri, 15 Aug 2025 at 22:12, Jan Kiszka <jan.kiszka@siemens.com> wrote:
> >>
> >> From: Jan Kiszka <jan.kiszka@siemens.com>
> >>
> >> 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@siemens.com>
> >> ---
> > 
> > [...]
> > 
> >>         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?
> 
> I was looking at tee_dyn_shm_alloc_helper and the dance it does to
> calculate the pages from the size parameter.

The rework of tee_shm_register_kernel_buf() is directly accept kernel
pages instead of buffer pointers is already due. If you are willing to
fix existing TEE client drivers and the API then I will be happy to
review them.

-Sumit