Patchwork uefi: uefidump: don't recurse forever on zero lengths (LP: #1174947)

login
register
mail settings
Submitter Colin King
Date May 3, 2013, 4:01 p.m.
Message ID <1367596915-9177-1-git-send-email-colin.king@canonical.com>
Download mbox | patch
Permalink /patch/241336/
State Accepted
Headers show

Comments

Colin King - May 3, 2013, 4:01 p.m.
From: Colin Ian King <colin.king@canonical.com>

We need to ensure that broken UEFI variables with zero length structs don't cause us
to recurse infinitely.  So break out early and don't recurse so we run out of stack.

Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
 src/uefi/uefidump/uefidump.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
Alex Hung - May 6, 2013, 11:48 a.m.
On 05/03/2013 12:01 PM, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> We need to ensure that broken UEFI variables with zero length structs don't cause us
> to recurse infinitely.  So break out early and don't recurse so we run out of stack.
>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
>   src/uefi/uefidump/uefidump.c | 6 ++++--
>   1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/src/uefi/uefidump/uefidump.c b/src/uefi/uefidump/uefidump.c
> index b115a62..31412c7 100644
> --- a/src/uefi/uefidump/uefidump.c
> +++ b/src/uefi/uefidump/uefidump.c
> @@ -385,8 +385,10 @@ static char *uefidump_build_dev_path(char *path, fwts_uefi_dev_path *dev_path)
>   	if (!((dev_path->type & 0x7f) == (FWTS_UEFI_END_DEV_PATH_TYPE) &&
>   	      (dev_path->subtype == FWTS_UEFI_END_ENTIRE_DEV_PATH_SUBTYPE))) {
>   		uint16_t len = dev_path->length[0] | (((uint16_t)dev_path->length[1])<<8);
> -		dev_path = (fwts_uefi_dev_path*)((char *)dev_path + len);
> -		path = uefidump_build_dev_path(path, dev_path);
> +		if (len > 0) {
> +			dev_path = (fwts_uefi_dev_path*)((char *)dev_path + len);
> +			path = uefidump_build_dev_path(path, dev_path);
> +		}
>   	}
>
>   	return path;
>
Acked-by: Alex Hung <alex.hung@canonical.com>
Ivan Hu - May 7, 2013, 2:14 a.m.
On 05/04/2013 12:01 AM, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> We need to ensure that broken UEFI variables with zero length structs don't cause us
> to recurse infinitely.  So break out early and don't recurse so we run out of stack.
>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
>   src/uefi/uefidump/uefidump.c | 6 ++++--
>   1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/src/uefi/uefidump/uefidump.c b/src/uefi/uefidump/uefidump.c
> index b115a62..31412c7 100644
> --- a/src/uefi/uefidump/uefidump.c
> +++ b/src/uefi/uefidump/uefidump.c
> @@ -385,8 +385,10 @@ static char *uefidump_build_dev_path(char *path, fwts_uefi_dev_path *dev_path)
>   	if (!((dev_path->type & 0x7f) == (FWTS_UEFI_END_DEV_PATH_TYPE) &&
>   	      (dev_path->subtype == FWTS_UEFI_END_ENTIRE_DEV_PATH_SUBTYPE))) {
>   		uint16_t len = dev_path->length[0] | (((uint16_t)dev_path->length[1])<<8);
> -		dev_path = (fwts_uefi_dev_path*)((char *)dev_path + len);
> -		path = uefidump_build_dev_path(path, dev_path);
> +		if (len > 0) {
> +			dev_path = (fwts_uefi_dev_path*)((char *)dev_path + len);
> +			path = uefidump_build_dev_path(path, dev_path);
> +		}
>   	}
>
>   	return path;
>

Acked-by: Ivan Hu <ivan.hu@canonical.com>

Patch

diff --git a/src/uefi/uefidump/uefidump.c b/src/uefi/uefidump/uefidump.c
index b115a62..31412c7 100644
--- a/src/uefi/uefidump/uefidump.c
+++ b/src/uefi/uefidump/uefidump.c
@@ -385,8 +385,10 @@  static char *uefidump_build_dev_path(char *path, fwts_uefi_dev_path *dev_path)
 	if (!((dev_path->type & 0x7f) == (FWTS_UEFI_END_DEV_PATH_TYPE) &&
 	      (dev_path->subtype == FWTS_UEFI_END_ENTIRE_DEV_PATH_SUBTYPE))) {
 		uint16_t len = dev_path->length[0] | (((uint16_t)dev_path->length[1])<<8);
-		dev_path = (fwts_uefi_dev_path*)((char *)dev_path + len);
-		path = uefidump_build_dev_path(path, dev_path);
+		if (len > 0) {
+			dev_path = (fwts_uefi_dev_path*)((char *)dev_path + len);
+			path = uefidump_build_dev_path(path, dev_path);
+		}
 	}
 
 	return path;