ACK: Fwd: [PATCH] uefirtvariable: Correct DataSize for GetVariable()

[Colin Ian King]

On 25/02/13 09:40, Colin Ian King wrote:
> Fwd, we should pick this patch up.
>
> Colin
>
> -------- Original Message --------
>
> X-Scanned-By: MailControl 13381.65 (www.mailcontrol.com) on 10.74.0.136
>
> From: Matt Fleming <matt.fleming at intel.com>
>
> When calling GetVariable(), we need to fill out DataSize with the size
> of the Data buffer instead of using whatever garbage value is left on
> the stack.
>
> Furthermore, the Data buffers are not strings in any of the tests, so
> there's no need to NUL-terminate them or make them 1-byte larger than
> datasize, e.g. there's no real need to do this,
>
>      uint8_t data[datasize+1;
>
> since the last element of data[] is never read back anyway.
>
> Cc: Ivan Hu <ivan.hu at canonical.com>
> Cc: Colin Ian King <colin.king at canonical.com>
> Cc: Alex Hung <alex.hung at canonical.com>
> Signed-off-by: Matt Fleming <matt.fleming at intel.com>
> ---
>
> Folks, is there a mailing list for the development of fwts?
>
>   src/uefi/uefirtvariable/uefirtvariable.c | 19 +++++++------------
>   1 file changed, 7 insertions(+), 12 deletions(-)
>
> diff --git a/src/uefi/uefirtvariable/uefirtvariable.c
> b/src/uefi/uefirtvariable/uefirtvariable.c
> index e8aa041..e00b343 100644
> --- a/src/uefi/uefirtvariable/uefirtvariable.c
> +++ b/src/uefi/uefirtvariable/uefirtvariable.c
> @@ -89,15 +89,14 @@ static int getvariable_test(fwts_framework *fw,
> uint64_t datasize, uint16_t *var
>       uint64_t status;
>       uint8_t testdata[MAX_DATA_LENGTH];
>       uint64_t dataindex;
> -    uint64_t getdatasize;
> +    uint64_t getdatasize = sizeof(testdata);
>       uint32_t attributestest;
>
> -    uint8_t data[datasize+1];
> +    uint8_t data[datasize];
>       uint32_t i;
>
>       for (dataindex = 0; dataindex < datasize; dataindex++)
>           data[dataindex] = (uint8_t)dataindex;
> -    data[dataindex] = '\0';
>
>       setvariable.VariableName = varname;
>       setvariable.VendorGuid = &gtestguid1;
> @@ -245,7 +244,6 @@ static int getnextvariable_test(fwts_framework *fw)
>
>       for (dataindex = 0; dataindex < datasize; dataindex++)
>           data[dataindex] = (uint8_t)dataindex;
> -    data[dataindex] = '\0';
>
>       setvariable.VariableName = variablenametest;
>       setvariable.VendorGuid = &gtestguid1;
> @@ -350,11 +348,10 @@ static int
> setvariable_insertvariable(fwts_framework *fw, uint32_t attributes, u
>       uint64_t status;
>       uint64_t dataindex;
>
> -    uint8_t data[datasize+1];
> +    uint8_t data[datasize];
>
>       for (dataindex = 0; dataindex < datasize; dataindex++)
>           data[dataindex] = (uint8_t)dataindex + datadiff;
> -    data[dataindex] = '\0';
>
>       setvariable.VariableName = varname;
>       setvariable.VendorGuid = gtestguid;
> @@ -392,9 +389,9 @@ static int setvariable_checkvariable(fwts_framework
> *fw, uint64_t datasize,
>       struct efi_getvariable getvariable;
>
>       uint64_t status;
> -    uint8_t testdata[datasize+1];
> +    uint8_t testdata[datasize];
>       uint64_t dataindex;
> -    uint64_t getdatasize;
> +    uint64_t getdatasize = sizeof(testdata);
>       uint32_t attributestest;
>
>       getvariable.VariableName = varname;
> @@ -442,7 +439,7 @@ static int
> setvariable_checkvariable_notfound(fwts_framework *fw, uint16_t *varn
>
>       uint64_t status;
>       uint8_t testdata[MAX_DATA_LENGTH];
> -    uint64_t getdatasize;
> +    uint64_t getdatasize = sizeof(testdata);
>       uint32_t attributestest;
>
>       getvariable.VariableName = varname;
> @@ -472,11 +469,10 @@ static int setvariable_invalidattr(fwts_framework
> *fw, uint32_t attributes, uint
>       struct efi_setvariable setvariable;
>       uint64_t status;
>       uint64_t dataindex;
> -    uint8_t data[datasize+1];
> +    uint8_t data[datasize];
>
>       for (dataindex = 0; dataindex < datasize; dataindex++)
>           data[dataindex] = (uint8_t)dataindex + datadiff;
> -    data[dataindex] = '\0';
>
>       setvariable.VariableName = varname;
>       setvariable.VendorGuid = gtestguid;
> @@ -775,7 +771,6 @@ static int getnextvariable_multitest(fwts_framework
> *fw, uint32_t multitesttime)
>
>       for (dataindex = 0; dataindex < datasize; dataindex++)
>           data[dataindex] = (uint8_t)dataindex;
> -    data[dataindex] = '\0';
>
>       setvariable.VariableName = variablenametest;
>       setvariable.VendorGuid = &gtestguid1;

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