[prev in list] [next in list] [prev in thread] [next in thread] 

List:       fwts-devel
Subject:    Re: [PATCH] uefirtvariable: modify getnextvariablename test for VariableNameSize smaller than 2 (LP:
From:       Colin Ian King <colin.king () canonical ! com>
Date:       2016-06-17 9:03:48
Message-ID: 5763BCF4.90002 () canonical ! com
[Download RAW message or body]

On 17/06/16 11:58, Ivan Hu wrote:
> Some firmwares (mostly on AMI Bios), the implementation limitation was seen
> that the VariableNameSize of the GetNextVariableName should lager or equal to 2.
> They believe that it should at least have a Null-terminated(2 bytes) for the
> name buffer.
> 
> So test getnextvariablename with VariableNameSize 0 and 1 will get the
> EFI_INVALID_PARAMETER, not expected EFI_BUFFER_TOO_SMALL.
> 
> UEFI spec doesn't describe it clearly, VariableName is the Null-treminated
> string, so the EFI_INVALID_PARAMETER seems also a resonable return when
> VariableNameSize smaller than 2.
> Before the spec has a clearly description, for the fwts getnextvariablename
> buffer too small test(getnextvariable_test4),
> 1. add the VariableNameSize 2 for the test, if VariableNameSize 2 without
> returning EFI_BUFFER_TOO_SMALL, then test fail.
> 2. if VariableNameSize is 0 or 1, allow the extra return EFI_INVALID_PARAMETER.
> 
> Signed-off-by: Ivan Hu <ivan.hu@canonical.com>
> ---
>  src/uefi/uefirtvariable/uefirtvariable.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/src/uefi/uefirtvariable/uefirtvariable.c b/src/uefi/uefirtvariable/uefirtvariable.c
> index 0514805..aca0202 100644
> --- a/src/uefi/uefirtvariable/uefirtvariable.c
> +++ b/src/uefi/uefirtvariable/uefirtvariable.c
> @@ -812,8 +812,7 @@ static int getnextvariable_test4(fwts_framework *fw)
>  		goto err;
>  	}
>  
> -	/* No first result can be 0 or 1 byte in size. */
> -	for (i = 0; i < 2; i++) {
> +	for (i = 0; i < 3; i++) {
>  		variablenamesize = i;
>  		getnextvariablename.VariableNameSize = &variablenamesize;
>  
> @@ -831,6 +830,8 @@ static int getnextvariable_test4(fwts_framework *fw)
>  		 * EFI_NOT_FOUND at this point.
>  		 */
>  		if (ioret != -1 || status != EFI_BUFFER_TOO_SMALL) {
> +			if (i != 2 && status == EFI_INVALID_PARAMETER)
> +				continue;
>  			fwts_failed(fw, LOG_LEVEL_HIGH,
>  				"UEFIRuntimeGetNextVariableName",
>  				"Expected EFI_BUFFER_TOO_SMALL with small "
> 
Given that the spec is not clear, this seems like the best approach.

Acked-by: Colin Ian King <colin.king@canonical.com>

-- 
fwts-devel mailing list
fwts-devel@lists.ubuntu.com
Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/fwts-devel
[prev in list] [next in list] [prev in thread] [next in thread]