From mboxrd@z Thu Jan  1 00:00:00 1970
From: Ilias Apalodimas <ilias.apalodimas@linaro.org>
Date: Wed, 13 May 2020 11:10:31 +0300
Subject: [PATCH 2/5 v2] efi_loader: Implement EFI variable handling via
 OP-TEE
In-Reply-To: <826c3b50-5115-855a-128e-37f000faf7b0@gmx.de>
References: <20200511181403.19448-1-ilias.apalodimas@linaro.org>
 <20200511181403.19448-3-ilias.apalodimas@linaro.org>
 <826c3b50-5115-855a-128e-37f000faf7b0@gmx.de>
Message-ID: <20200513081031.GA100468@apalos.home>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

On Wed, May 13, 2020 at 08:14:19AM +0200, Heinrich Schuchardt wrote:
> On 5/11/20 8:14 PM, Ilias Apalodimas wrote:
> > In OP-TEE we can run EDK2's StandAloneMM on a secure partition.
> > StandAloneMM is responsible for the UEFI variable support. In
> > +

[...]

> > +	EFI_ENTRY("%p \"%ls\" %pUl", variable_name_size, variable_name, guid);
> > +
> > +	if (!variable_name_size || !variable_name || !guid)
> > +		return EFI_EXIT(EFI_INVALID_PARAMETER);
> > +
> > +	out_name_size = *variable_name_size;
> > +	in_name_size = u16_strsize(variable_name);
> 
> The UEFI spec requires that EFI_INVALID_PARAMETER should be returned if
> there is no '\0' character in the first *variable_name_size words of
> variable_name. I think we should add this test here instead of using
> max(out_name_size, in_name_size) later in the code.

Ok I'll have a look 

> 
> You are currently calling EFI_EXIT() in many places. Depending on the
> level of code optimizations done by the compiler and the debug settings
> this may need to unnecessary code size. I suggest to use a single exit
> point in each of the functions, e.g.
> 
> 	if (out_name_size > in_name_size) {
> 		ret = EFI_INVALID_PARAMETER;
> 		goto out;
> 	}
> ...
> out:
> 	EFI_EXIT(ret);

Fair enough, most of U-Boot is coded that way anyway, might as well have a
common approach.

I'll post a v3 with the changes, so you can do your testing directly in that 


Regards
/Ilias