| Message ID | 20211215075000.1885137-1-ilias.apalodimas@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | efi_loader: Bump the number of shared pages with StandAloneMM | expand |
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? Best regards Heinrich > /* > * There seems to be a bug in EDK2 miscalculating the boundaries and
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
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; /* * There seems to be a bug in EDK2 miscalculating the boundaries and