[PATCH v2] efi_loader: Don't limit the StMM buffer size explicitly

[Ilias Apalodimas]

On Sat, 25 Dec 2021, 16:28 Heinrich Schuchardt, <xypron.glpk at gmx.de> wrote:

>
>
> Am 25. Dezember 2021 12:16:29 MEZ schrieb Ilias Apalodimas <
> ilias.apalodimas at linaro.org>:
> >> >
> >[...]
> >> >    rc = tee_invoke_func(conn.tee, &arg, 2, param);
> >> >    tee_shm_free(shm);
> >> > +  /*
> >> > +   * Although the max payload is configurable on StMM, we only share
> >> > +   * four pages from OP-TEE for the non-secure buffer used to
> communicate
> >> > +   * with StMM. OP-TEE will reject anything bigger than that and will
> >> > +   * return.  So le'ts at least warn users
> >> > +   */
> >> >    tee_close_session(conn.tee, conn.session);
> >> > -  if (rc || arg.ret != TEE_SUCCESS)
> >> > +  if (rc || arg.ret != TEE_SUCCESS) {
> >>
> >> tee_close_session(): Will arg.ret be valid if rc != 0?
> >
> >Depends when tee_invoke_func() failed.  But why do we care?
>
> Should we write a message if rc !=0 && arg.ret == TEE_ERROR_EXCESS_DATA?
>

I don't think its needed. OPTEE will set that only if RC == 0

Cheers
Ilias

>
> >The connection needs to close regardless and we then have to reason with
> >the error.
> >
> >Regards
> >/Ilias
> >>
> >> Best regards
> >>
> >> Heinrich
> >>
> >> > +          if (arg.ret == TEE_ERROR_EXCESS_DATA)
> >> > +                  log_err("Variable payload too large\n");
> >> >            return EFI_DEVICE_ERROR;
> >> > +  }
> >> >
> >> >    switch (param[1].u.value.a) {
> >> >    case ARM_SVC_SPM_RET_SUCCESS:
> >> > @@ -255,15 +263,6 @@ efi_status_t EFIAPI get_max_payload(efi_uintn_t
> *size)
> >> >            goto out;
> >> >    }
> >> >    *size = var_payload->size;
> >> > -  /*
> >> > -   * Although the max payload is configurable on StMM, we only share
> a
> >> > -   * single page from OP-TEE for the non-secure buffer used to
> communicate
> >> > -   * 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  -
> >> > -                  MM_VARIABLE_COMMUNICATE_SIZE;
> >> >    /*
> >> >     * There seems to be a bug in EDK2 miscalculating the boundaries
> and
> >> >     * size checks, so deduct 2 more bytes to fulfill this
> requirement. Fix
> >>
>