Re: [U-Boot] [PATCH] efi_loader: fix off-by-one bug in efi_get_variable

[Heinrich Schuchardt]

On 05/09/2018 12:50 AM, 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 


Thanks for the patch.

There other issues we might want to fix:

If the blob has an uneven number of hexadecimal digits 2 N + 1 the 
function hex2mem is called with count = 2 N + 2. hex('\0') will return 
-1, hex2mem returns NULL, and the blob is happily considered as correct. 
We should create an error instead.


There is no need for the argument count at all as hexstr is '\0' terminated.


---
  lib/efi_loader/efi_variable.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c
index 6c177da..d031338 100644
--- a/lib/efi_loader/efi_variable.c
+++ b/lib/efi_loader/efi_variable.c
@@ -68,11 +68,11 @@ static const char *hex2mem(u8 *mem, const char *hexstr, int 
count)
do {
int nibble;
  
-		*mem = 0;

-
if (!count || !*hexstr)
break;
  
+		*mem = 0;

+


Why should we have this line at all? We set *mem = nibble below.

Regards

Heinrich


nibble = hex(*hexstr);
if (nibble < 0)
break;


___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH] efi_loader: fix off-by-one bug in efi_get_variable

[Alexander Graf]

On 05/09/2018 12:50 AM, 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 


Thanks to the memset right above the loop we can just remove the *mem = 
0 line altogether, no?


Alex


---
  lib/efi_loader/efi_variable.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c
index 6c177da..d031338 100644
--- a/lib/efi_loader/efi_variable.c
+++ b/lib/efi_loader/efi_variable.c
@@ -68,11 +68,11 @@ static const char *hex2mem(u8 *mem, const char *hexstr, int 
count)
do {
int nibble;
  
-		*mem = 0;

-
if (!count || !*hexstr)
break;
  
+		*mem = 0;

+
nibble = hex(*hexstr);
if (nibble < 0)
break;



___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

[U-Boot] [PATCH] efi_loader: fix off-by-one bug in efi_get_variable

[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 
---
 lib/efi_loader/efi_variable.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c
index 6c177da..d031338 100644
--- a/lib/efi_loader/efi_variable.c
+++ b/lib/efi_loader/efi_variable.c
@@ -68,11 +68,11 @@ static const char *hex2mem(u8 *mem, const char *hexstr, int 
count)
do {
int nibble;
 
-   *mem = 0;
-
if (!count || !*hexstr)
break;
 
+   *mem = 0;
+
nibble = hex(*hexstr);
if (nibble < 0)
break;
-- 
2.7.4

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot