On 05/11/2018 07:54 PM, Ivan Gorinov wrote:
> efi_get_variable() always stores an extra zero byte after the output data.
> When the returned data size matches the output buffer size, the extra zero
> byte is stored past the end of the output buffer.
>
> Signed-off-by: Ivan Gorinov <ivan.gori...@intel.com>
> ---
> lib/efi_loader/efi_variable.c | 40 +++++++++++++++++-----------------------
> 1 file changed, 17 insertions(+), 23 deletions(-)
>
> diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c
> index 6c177da..28b2f5c 100644
> --- a/lib/efi_loader/efi_variable.c
> +++ b/lib/efi_loader/efi_variable.c
> @@ -50,7 +50,7 @@
> (strlen("efi_xxxxxxxx-xxxx-xxxx-xxxxxxxxxxxxxxxx_") + \
> (MAX_VAR_NAME * MAX_UTF8_PER_UTF16))
>
> -static int hex(unsigned char ch)
> +static int hex(int ch)
> {
> if (ch >= 'a' && ch <= 'f')
> return ch-'a'+10;
> @@ -61,44 +61,34 @@ static int hex(unsigned char ch)
> return -1;
> }
>
> -static const char *hex2mem(u8 *mem, const char *hexstr, int count)
> +static int hex2mem(u8 *mem, const char *hexstr, int size)
> {
> - memset(mem, 0, count/2);
> + int nibble;
> + int i;
>
> - do {
> - int nibble;
> + memset(mem, 0, size);
Why should we call memset?
The memory is supplied by the caller of efi_get_variable().
Either we copy size bytes to mem or we return EFI_DEVICE_ERROR.
>
> - *mem = 0;
> -
> - if (!count || !*hexstr)
> + for (i = 0; i < size; i++) {
> + if (*hexstr == '\0')
> break;
>
> nibble = hex(*hexstr);
> if (nibble < 0)
> - break;
> + return -1;
>
> *mem = nibble;
> - count--;
> hexstr++;
>
> - if (!count || !*hexstr)
> - break;
> -
> nibble = hex(*hexstr);
> if (nibble < 0)
> - break;
> + return -1;
>
> *mem = (*mem << 4) | nibble;
> - count--;
> hexstr++;
> mem++;
> + }
>
> - } while (1);
> -
> - if (*hexstr)
> - return hexstr;
> -
> - return NULL;
> + return i;
> }
>
> static char *mem2hex(char *hexstr, const u8 *mem, int count)
> @@ -210,8 +200,12 @@ efi_status_t EFIAPI efi_get_variable(s16 *variable_name,
> if ((s = prefix(val, "(blob)"))) {
> unsigned len = strlen(s);
>
> + /* number of hexadecimal digits must be even */
> + if (len & 1)
> + return EFI_EXIT(EFI_DEVICE_ERROR);
> +
> /* two characters per byte: */
> - len = DIV_ROUND_UP(len, 2);
> + len /= 2;
You do not catch the case that len & 1 == 1 which should cause
EFI_DEVICE_ERROR because the data in the blob is not valid.
Best regards
Heinrich
> *data_size = len;
>
> if (in_size < len)
> @@ -220,7 +214,7 @@ efi_status_t EFIAPI efi_get_variable(s16 *variable_name,
> if (!data)
> return EFI_EXIT(EFI_INVALID_PARAMETER);
>
> - if (hex2mem(data, s, len * 2))
> + if (hex2mem(data, s, len) != len)
> return EFI_EXIT(EFI_DEVICE_ERROR);
>
> debug("%s: got value: \"%s\"\n", __func__, s);
>
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
