Re: [PATCH] efi_loader: Bump the number of shared pages with StandAloneMM

[Ilias Apalodimas <ilias.apalodimas@linaro.org>]

Hi Heinrich,

On Sat, Dec 18, 2021 at 12:03:34PM +0100, Heinrich Schuchardt wrote:
> 
> 
> On 12/15/21 08:50, Ilias Apalodimas wrote:
> > Currently we allow (and explicitly check) a single shared page with
> > StandAloneMM.  This is dictated by OP-TEE which runs the application.
> > However there's no way for us dynamically discover the number of pages we
> > are allowed to use.  Since writing big EFI signature list variables
> > requires more than a page, OP-TEE has bumped the number of shared pages to
> > four.  Bump our page checks to four as well.
> > 
> > Note here that checking some kind of version and reason with the
> > compatibility doesn't make too much sense.  We sanitize the number of pages
> > internally in our U-Boot code but eventually OP-TEE will fail if we try to
> > write more than it's allowing. The error will just happen later on when we
> > access StandAloneMM.  So in order to avoid compatibility checks change the
> > number to four unconditionally.
> > 
> > Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> > Tested-by: Ying-Chun Liu (PaulLiu) <paul.liu@linaro.org>
> > ---
> >   lib/efi_loader/efi_variable_tee.c | 4 ++--
> >   1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/lib/efi_loader/efi_variable_tee.c b/lib/efi_loader/efi_variable_tee.c
> > index 281f886124af..95eaeaa5fd9d 100644
> > --- a/lib/efi_loader/efi_variable_tee.c
> > +++ b/lib/efi_loader/efi_variable_tee.c
> > @@ -261,8 +261,8 @@ efi_status_t EFIAPI get_max_payload(efi_uintn_t *size)
> >   	 * with StMM. Since OP-TEE will reject to map anything bigger than that,
> >   	 * make sure we are in bounds.
> >   	 */
> > -	if (*size > OPTEE_PAGE_SIZE)
> > -		*size = OPTEE_PAGE_SIZE - MM_COMMUNICATE_HEADER_SIZE  -
> > +	if (*size > 4 * OPTEE_PAGE_SIZE)
> > +		*size = 4 * OPTEE_PAGE_SIZE - MM_COMMUNICATE_HEADER_SIZE  -
> >   			MM_VARIABLE_COMMUNICATE_SIZE;
> 
> Why do we need this check at all if OPTEE checks again?
> 

OP-TEE will have to try and register the memory in tee_shm_register() to
fail. So since we know if only allows 4 pages we have an internal sanity checking
to bail out earlier.


Regards
/Ilias
> Best regards
> 
> Heinrich
> 
> >   	/*
> >   	 * There seems to be a bug in EDK2 miscalculating the boundaries and