efi_loader: fix off-by-one bug in efi_get_variable
PATCH v2 1/1 efi_loader: fix off-by-one bug in efi_get_variable
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 <[email protected]>
--- lib/efi_loader/efi_variable.c | 64 +++++++++++++++++++++---------------------- 1 file changed, 32 insertions(+), 32 deletions(-)
lib/efi_loader/efi_variable.c +32 -32
@@ -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 (char ch) {if (ch >='a' && ch <='f' )return ch-'a' +10 ;
@@ -61,44 +61,41 @@ 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 ); +u8 data; +int nibble; +char ch; +int i, k; -do { -int nibble; +memset (mem,0 , size); - *mem =0 ; +for (i =0 ; i < size; i +=1 ) { + data =0 ; +for (k =0 ; k <2 ; k +=1 ) { + ch = *(hexstr++); -if (!count || !*hexstr) -break ; - - nibble =hex (*hexstr); -if (nibble <0 ) -break ; - - *mem = nibble; - count--; - hexstr++; - -if (!count || !*hexstr) -break ; +if (!ch) { +if (k ==0 ) +return i; +else +return -1 ; + } - nibble =hex (*hexstr); -if (nibble <0 ) -break ; + nibble =hex (ch); +if (nibble <0 ) +return -1 ; - *mem = (*mem <<4 ) | nibble; - count--; - hexstr++; - mem++; + hexstr++; - }while (1 ); + data <<=4 ; + data |= nibble; -if (*hexstr) -return hexstr; + } + mem[i] = data; + } -return NULL ; +return i; }static char *mem2hex (char *hexstr,const u8 *mem,int count)
@@ -210,8 +207,11 @@ efi_status_t EFIAPI efi_get_variable(s16 *variable_name,if ((s =prefix (val,"(blob)" ))) {unsigned len =strlen (s); +if (len &1 ) +return EFI_EXIT (EFI_DEVICE_ERROR ); +/* two characters per byte: */ - len =DIV_ROUND_UP (len,2 ); + len /=2 ; *data_size = len;if (in_size < len)
@@ -220,7 +220,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);
Signature · 1 line
2.7.4
PATCH v2 1/1 efi_loader: fix off-by-one bug in efi_get_variable
Quoted 23 lines · Ivan Gorinov
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 <[email protected]>--- lib/efi_loader/efi_variable.c | 64 +++++++++++++++++++++---------------------- 1 file changed, 32 insertions(+), 32 deletions(-)lib/efi_loader/efi_variable.c +1
-1@@ -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 (char ch) {if (ch >='a' && ch <='f' )return ch-'a' +10 ;
lib/efi_loader/efi_variable.c static int hex(unsigned char ch) · line 64
Quoted 5 lines
lib/efi_loader/efi_variable.c +1
-1@@ -61,44 +61,41 @@ 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)
Why do you need size if hexstr is zero terminated?
lib/efi_loader/efi_variable.c static int hex(unsigned char ch) · line 73
Quoted 13 lines
lib/efi_loader/efi_variable.c +6
-4{ -memset (mem,0 , count/2 ); +u8 data; +int nibble; +char ch; +int i, k; - do { - int nibble; +memset (mem,0 , size); - *mem = 0; +for (i =0 ; i < size; i +=1 ) {
People are used to reading ++i or i++ for simple incrementation.
lib/efi_loader/efi_variable.c static int hex(unsigned char ch) · line 75
Quoted 2 lines
lib/efi_loader/efi_variable.c +2
-0+ data =0 ; +for (k =0 ; k <2 ; k +=1 ) {
Why use nested loops? That neither makes the code easier to read nor faster.
Why all the changes if you still do not throw an error if hexstr has an uneven number of nibbles?
Best regards
Heinrich
Quoted 73 lines
lib/efi_loader/efi_variable.c +22
-26+ ch = *(hexstr++); -if (!count || !*hexstr) -break ; - - nibble =hex (*hexstr); -if (nibble <0 ) -break ; - - *mem = nibble; - count--; - hexstr++; - -if (!count || !*hexstr) -break ; +if (!ch) { +if (k ==0 ) +return i; +else +return -1 ; + } - nibble =hex (*hexstr); -if (nibble <0 ) -break ; + nibble =hex (ch); +if (nibble <0 ) +return -1 ; - *mem = (*mem <<4 ) | nibble; - count--; - hexstr++; - mem++; + hexstr++; - }while (1 ); + data <<=4 ; + data |= nibble; -if (*hexstr) -return hexstr; + } + mem[i] = data; + } -return NULL ; +return i; }static char *mem2hex (char *hexstr,const u8 *mem,int count)@@ -210,8 +207,11 @@ efi_status_t EFIAPI efi_get_variable(s16 *variable_name,if ((s =prefix (val,"(blob)" ))) {unsigned len =strlen (s); +if (len &1 ) +return EFI_EXIT (EFI_DEVICE_ERROR ); +/* two characters per byte: */ - len =DIV_ROUND_UP (len,2 ); + len /=2 ; *data_size = len;if (in_size < len)@@ -220,7 +220,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);