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

[Heinrich Schuchardt]

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?

>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
>>