public inbox for kexec@lists.infradead.org
 help / color / mirror / Atom feed
* [PATCH] kexec/powerpc: Handle buffer overflow in kernel command line
@ 2013-04-02  8:03 Suzuki K. Poulose
  2013-04-02 10:58 ` Zhang Yanfei
  0 siblings, 1 reply; 5+ messages in thread
From: Suzuki K. Poulose @ 2013-04-02  8:03 UTC (permalink / raw)
  To: horms; +Cc: kexec

From: Suzuki K. Poulose <suzuki@in.ibm.com>

Enforce size check for kernel command line to make sure it
doesn't overflow COMMAND_LINE_SIZE.

Reported-by: Nathan D. Miller <nathanm2@us.ibm.com>
Signed-off-by: Suzuki K. Poulose <suzuki@in.ibm.com>
---
 kexec/arch/ppc/kexec-elf-ppc.c    |   41 ++++++++++++++++++-------------------
 kexec/arch/ppc/kexec-uImage-ppc.c |   31 ++++++++++++++--------------
 2 files changed, 35 insertions(+), 37 deletions(-)

diff --git a/kexec/arch/ppc/kexec-elf-ppc.c b/kexec/arch/ppc/kexec-elf-ppc.c
index 5f63a64..7ba8421 100644
--- a/kexec/arch/ppc/kexec-elf-ppc.c
+++ b/kexec/arch/ppc/kexec-elf-ppc.c
@@ -156,7 +156,7 @@ int elf_ppc_load(int argc, char **argv,	const char *buf, off_t len,
 {
 	struct mem_ehdr ehdr;
 	char *command_line, *crash_cmdline, *cmdline_buf;
-	int command_line_len;
+	int command_line_len, crash_cmdline_len;
 	char *dtb;
 	int result;
 	unsigned long max_addr, hole_addr;
@@ -233,27 +233,15 @@ int elf_ppc_load(int argc, char **argv,	const char *buf, off_t len,
 	}
 
 	command_line_len = 0;
-	if (command_line) {
-		command_line_len = strlen(command_line) + 1;
-	} else {
+	if (!command_line)
 		command_line = get_command_line();
-		command_line_len = strlen(command_line) + 1;
-	}
+	command_line_len = strlen(command_line);
 
 	if (ramdisk && reuse_initrd)
 		die("Can't specify --ramdisk or --initrd with --reuseinitrd\n");
 
 	fixup_nodes[cur_fixup] = NULL;
 
-	/* Need to append some command line parameters internally in case of
-	 * taking crash dumps.
-	 */
-	if (info->kexec_flags & KEXEC_ON_CRASH) {
-		crash_cmdline = xmalloc(COMMAND_LINE_SIZE);
-		memset((void *)crash_cmdline, 0, COMMAND_LINE_SIZE);
-	} else
-		crash_cmdline = NULL;
-
 	/* Parse the Elf file */
 	result = build_elf_exec_info(buf, len, &ehdr, 0);
 	if (result < 0) {
@@ -291,26 +279,37 @@ int elf_ppc_load(int argc, char **argv,	const char *buf, off_t len,
 		return result;
 	}
 
-	/* If panic kernel is being loaded, additional segments need
-	 * to be created.
+	/*
+	 * Need to append some command line parameters internally in case of
+	 * taking crash dumps. Additional segments have to be loaded for panic
+	 * kernel.
 	 */
 	if (info->kexec_flags & KEXEC_ON_CRASH) {
+		crash_cmdline = xmalloc(COMMAND_LINE_SIZE);
+		memset((void *)crash_cmdline, 0, COMMAND_LINE_SIZE);
 		result = load_crashdump_segments(info, crash_cmdline,
 						max_addr, 0);
 		if (result < 0) {
 			free(crash_cmdline);
 			return -1;
 		}
+		crash_cmdline_len = strlen(crash_cmdline);
+	} else {
+		crash_cmdline = NULL;
+		crash_cmdline_len = 0;
 	}
 
+	if (crash_cmdline_len + command_line_len + 1 > COMMAND_LINE_SIZE) {
+		printf ("Command line buffer overflow\n");
+		return -1;
+	}
+		
 	cmdline_buf = xmalloc(COMMAND_LINE_SIZE);
 	memset((void *)cmdline_buf, 0, COMMAND_LINE_SIZE);
 	if (command_line)
-		strncat(cmdline_buf, command_line, command_line_len);
+		strcpy(cmdline_buf, command_line);
 	if (crash_cmdline)
-		strncat(cmdline_buf, crash_cmdline,
-				sizeof(crash_cmdline) -
-				strlen(crash_cmdline) - 1);
+		strncat(cmdline_buf, crash_cmdline, crash_cmdline_len);
 
 	/*
 	 * In case of a toy we take the hardcoded things and an easy setup via
diff --git a/kexec/arch/ppc/kexec-uImage-ppc.c b/kexec/arch/ppc/kexec-uImage-ppc.c
index 900cd16..0c96bac 100644
--- a/kexec/arch/ppc/kexec-uImage-ppc.c
+++ b/kexec/arch/ppc/kexec-uImage-ppc.c
@@ -81,7 +81,7 @@ static int ppc_load_bare_bits(int argc, char **argv, const char *buf,
 		unsigned int ep)
 {
 	char *command_line, *cmdline_buf, *crash_cmdline;
-	int command_line_len;
+	int command_line_len, crash_cmdline_len;
 	char *dtb;
 	unsigned int addr;
 	unsigned long dtb_addr;
@@ -140,13 +140,10 @@ static int ppc_load_bare_bits(int argc, char **argv, const char *buf,
 		die("Can't specify --ramdisk or --initrd with --reuseinitrd\n");
 
 	command_line_len = 0;
-	if (command_line) {
-		command_line_len = strlen(command_line) + 1;
-	} else {
+	if (!command_line)
 		command_line = get_command_line();
-		command_line_len = strlen(command_line) + 1;
-	}
 
+	command_line_len = strlen(command_line);
 	fixup_nodes[cur_fixup] = NULL;
 
 	/*
@@ -179,25 +176,27 @@ static int ppc_load_bare_bits(int argc, char **argv, const char *buf,
 	if (info->kexec_flags & KEXEC_ON_CRASH) {
                 crash_cmdline = xmalloc(COMMAND_LINE_SIZE);
                 memset((void *)crash_cmdline, 0, COMMAND_LINE_SIZE);
-        } else
-                crash_cmdline = NULL;
-
-	if (info->kexec_flags & KEXEC_ON_CRASH) {
 		ret = load_crashdump_segments(info, crash_cmdline,
 						max_addr, 0);
-		if (ret < 0) {
+		if (ret < 0)
 			return -1;
-		}
+		crash_cmdline_len = strlen(crash_cmdline);
+	} else {
+		crash_cmdline = NULL;
+		crash_cmdline_len = 0;
+	}
+
+	if (command_line_len + crash_cmdline_len + 1 > COMMAND_LINE_SIZE) {
+		printf("Command line buffer overflow \n");
+		return -1;
 	}
 
 	cmdline_buf = xmalloc(COMMAND_LINE_SIZE);
 	memset((void *)cmdline_buf, 0, COMMAND_LINE_SIZE);
 	if (command_line)
-		strncat(cmdline_buf, command_line, command_line_len);
+		strcpy(cmdline_buf, command_line);
 	if (crash_cmdline)
-		strncat(cmdline_buf, crash_cmdline,
-			sizeof(crash_cmdline) -
-			strlen(crash_cmdline) - 1);
+		strncat(cmdline_buf, crash_cmdline, crash_cmdline_len);
 
 	elf_rel_build_load(info, &info->rhdr, (const char *)purgatory,
 				purgatory_size, 0, -1, -1, 0);


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] kexec/powerpc: Handle buffer overflow in kernel command line
  2013-04-02  8:03 [PATCH] kexec/powerpc: Handle buffer overflow in kernel command line Suzuki K. Poulose
@ 2013-04-02 10:58 ` Zhang Yanfei
  2013-04-02 12:13   ` [UPDATED PATCH] " Suzuki K. Poulose
  0 siblings, 1 reply; 5+ messages in thread
From: Zhang Yanfei @ 2013-04-02 10:58 UTC (permalink / raw)
  To: Suzuki K. Poulose; +Cc: horms, kexec

Hello Suzuki,

Conflicts occur when I tried to apply the patch to latest kexec-tools by
using git am.

Applying: kexec/powerpc: Handle buffer overflow in kernel command line
/data/kexec-tools/.git/rebase-apply/patch:79: trailing whitespace.
		
error: patch failed: kexec/arch/ppc/kexec-elf-ppc.c:156
error: kexec/arch/ppc/kexec-elf-ppc.c: patch does not apply
error: patch failed: kexec/arch/ppc/kexec-uImage-ppc.c:81
error: kexec/arch/ppc/kexec-uImage-ppc.c: patch does not apply
Patch failed at 0001 kexec/powerpc: Handle buffer overflow in kernel command line
When you have resolved this problem run "git am --resolved".
If you would prefer to skip this patch, instead run "git am --skip".
To restore the original branch and stop patching run "git am --abort".

Thanks
Zhang

于 2013年04月02日 16:03, Suzuki K. Poulose 写道:
> From: Suzuki K. Poulose <suzuki@in.ibm.com>
> 
> Enforce size check for kernel command line to make sure it
> doesn't overflow COMMAND_LINE_SIZE.
> 
> Reported-by: Nathan D. Miller <nathanm2@us.ibm.com>
> Signed-off-by: Suzuki K. Poulose <suzuki@in.ibm.com>
> ---
>  kexec/arch/ppc/kexec-elf-ppc.c    |   41 ++++++++++++++++++-------------------
>  kexec/arch/ppc/kexec-uImage-ppc.c |   31 ++++++++++++++--------------
>  2 files changed, 35 insertions(+), 37 deletions(-)
> 
> diff --git a/kexec/arch/ppc/kexec-elf-ppc.c b/kexec/arch/ppc/kexec-elf-ppc.c
> index 5f63a64..7ba8421 100644
> --- a/kexec/arch/ppc/kexec-elf-ppc.c
> +++ b/kexec/arch/ppc/kexec-elf-ppc.c
> @@ -156,7 +156,7 @@ int elf_ppc_load(int argc, char **argv,	const char *buf, off_t len,
>  {
>  	struct mem_ehdr ehdr;
>  	char *command_line, *crash_cmdline, *cmdline_buf;
> -	int command_line_len;
> +	int command_line_len, crash_cmdline_len;
>  	char *dtb;
>  	int result;
>  	unsigned long max_addr, hole_addr;
> @@ -233,27 +233,15 @@ int elf_ppc_load(int argc, char **argv,	const char *buf, off_t len,
>  	}
>  
>  	command_line_len = 0;
> -	if (command_line) {
> -		command_line_len = strlen(command_line) + 1;
> -	} else {
> +	if (!command_line)
>  		command_line = get_command_line();
> -		command_line_len = strlen(command_line) + 1;
> -	}
> +	command_line_len = strlen(command_line);
>  
>  	if (ramdisk && reuse_initrd)
>  		die("Can't specify --ramdisk or --initrd with --reuseinitrd\n");
>  
>  	fixup_nodes[cur_fixup] = NULL;
>  
> -	/* Need to append some command line parameters internally in case of
> -	 * taking crash dumps.
> -	 */
> -	if (info->kexec_flags & KEXEC_ON_CRASH) {
> -		crash_cmdline = xmalloc(COMMAND_LINE_SIZE);
> -		memset((void *)crash_cmdline, 0, COMMAND_LINE_SIZE);
> -	} else
> -		crash_cmdline = NULL;
> -
>  	/* Parse the Elf file */
>  	result = build_elf_exec_info(buf, len, &ehdr, 0);
>  	if (result < 0) {
> @@ -291,26 +279,37 @@ int elf_ppc_load(int argc, char **argv,	const char *buf, off_t len,
>  		return result;
>  	}
>  
> -	/* If panic kernel is being loaded, additional segments need
> -	 * to be created.
> +	/*
> +	 * Need to append some command line parameters internally in case of
> +	 * taking crash dumps. Additional segments have to be loaded for panic
> +	 * kernel.
>  	 */
>  	if (info->kexec_flags & KEXEC_ON_CRASH) {
> +		crash_cmdline = xmalloc(COMMAND_LINE_SIZE);
> +		memset((void *)crash_cmdline, 0, COMMAND_LINE_SIZE);
>  		result = load_crashdump_segments(info, crash_cmdline,
>  						max_addr, 0);
>  		if (result < 0) {
>  			free(crash_cmdline);
>  			return -1;
>  		}
> +		crash_cmdline_len = strlen(crash_cmdline);
> +	} else {
> +		crash_cmdline = NULL;
> +		crash_cmdline_len = 0;
>  	}
>  
> +	if (crash_cmdline_len + command_line_len + 1 > COMMAND_LINE_SIZE) {
> +		printf ("Command line buffer overflow\n");
> +		return -1;
> +	}
> +		
>  	cmdline_buf = xmalloc(COMMAND_LINE_SIZE);
>  	memset((void *)cmdline_buf, 0, COMMAND_LINE_SIZE);
>  	if (command_line)
> -		strncat(cmdline_buf, command_line, command_line_len);
> +		strcpy(cmdline_buf, command_line);
>  	if (crash_cmdline)
> -		strncat(cmdline_buf, crash_cmdline,
> -				sizeof(crash_cmdline) -
> -				strlen(crash_cmdline) - 1);
> +		strncat(cmdline_buf, crash_cmdline, crash_cmdline_len);
>  
>  	/*
>  	 * In case of a toy we take the hardcoded things and an easy setup via
> diff --git a/kexec/arch/ppc/kexec-uImage-ppc.c b/kexec/arch/ppc/kexec-uImage-ppc.c
> index 900cd16..0c96bac 100644
> --- a/kexec/arch/ppc/kexec-uImage-ppc.c
> +++ b/kexec/arch/ppc/kexec-uImage-ppc.c
> @@ -81,7 +81,7 @@ static int ppc_load_bare_bits(int argc, char **argv, const char *buf,
>  		unsigned int ep)
>  {
>  	char *command_line, *cmdline_buf, *crash_cmdline;
> -	int command_line_len;
> +	int command_line_len, crash_cmdline_len;
>  	char *dtb;
>  	unsigned int addr;
>  	unsigned long dtb_addr;
> @@ -140,13 +140,10 @@ static int ppc_load_bare_bits(int argc, char **argv, const char *buf,
>  		die("Can't specify --ramdisk or --initrd with --reuseinitrd\n");
>  
>  	command_line_len = 0;
> -	if (command_line) {
> -		command_line_len = strlen(command_line) + 1;
> -	} else {
> +	if (!command_line)
>  		command_line = get_command_line();
> -		command_line_len = strlen(command_line) + 1;
> -	}
>  
> +	command_line_len = strlen(command_line);
>  	fixup_nodes[cur_fixup] = NULL;
>  
>  	/*
> @@ -179,25 +176,27 @@ static int ppc_load_bare_bits(int argc, char **argv, const char *buf,
>  	if (info->kexec_flags & KEXEC_ON_CRASH) {
>                  crash_cmdline = xmalloc(COMMAND_LINE_SIZE);
>                  memset((void *)crash_cmdline, 0, COMMAND_LINE_SIZE);
> -        } else
> -                crash_cmdline = NULL;
> -
> -	if (info->kexec_flags & KEXEC_ON_CRASH) {
>  		ret = load_crashdump_segments(info, crash_cmdline,
>  						max_addr, 0);
> -		if (ret < 0) {
> +		if (ret < 0)
>  			return -1;
> -		}
> +		crash_cmdline_len = strlen(crash_cmdline);
> +	} else {
> +		crash_cmdline = NULL;
> +		crash_cmdline_len = 0;
> +	}
> +
> +	if (command_line_len + crash_cmdline_len + 1 > COMMAND_LINE_SIZE) {
> +		printf("Command line buffer overflow \n");
> +		return -1;
>  	}
>  
>  	cmdline_buf = xmalloc(COMMAND_LINE_SIZE);
>  	memset((void *)cmdline_buf, 0, COMMAND_LINE_SIZE);
>  	if (command_line)
> -		strncat(cmdline_buf, command_line, command_line_len);
> +		strcpy(cmdline_buf, command_line);
>  	if (crash_cmdline)
> -		strncat(cmdline_buf, crash_cmdline,
> -			sizeof(crash_cmdline) -
> -			strlen(crash_cmdline) - 1);
> +		strncat(cmdline_buf, crash_cmdline, crash_cmdline_len);
>  
>  	elf_rel_build_load(info, &info->rhdr, (const char *)purgatory,
>  				purgatory_size, 0, -1, -1, 0);
> 
> 
> _______________________________________________
> kexec mailing list
> kexec@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/kexec


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [UPDATED PATCH] kexec/powerpc: Handle buffer overflow in kernel command line
  2013-04-02 10:58 ` Zhang Yanfei
@ 2013-04-02 12:13   ` Suzuki K. Poulose
  2013-04-16  6:17     ` Suzuki K. Poulose
  0 siblings, 1 reply; 5+ messages in thread
From: Suzuki K. Poulose @ 2013-04-02 12:13 UTC (permalink / raw)
  To: horms; +Cc: kexec, zhangyanfei.yes

Zhang,

Sorry, it was my mistake. I have updated my patch based on the upstream
tree now.

---

From: Suzuki K. Poulose <suzuki@in.ibm.com>

Enforce size check for kernel command line to make sure it
doesn't overflow COMMAND_LINE_SIZE.

Reported-by: Nathan D. Miller <nathanm2@us.ibm.com>
Signed-off-by: Suzuki K. Poulose <suzuki@in.ibm.com>
---
 kexec/arch/ppc/kexec-elf-ppc.c    |   29 ++++++++++++++++-------------
 kexec/arch/ppc/kexec-uImage-ppc.c |   23 ++++++++++++++---------
 2 files changed, 30 insertions(+), 22 deletions(-)

diff --git a/kexec/arch/ppc/kexec-elf-ppc.c b/kexec/arch/ppc/kexec-elf-ppc.c
index 3daca2d..98cae9c 100644
--- a/kexec/arch/ppc/kexec-elf-ppc.c
+++ b/kexec/arch/ppc/kexec-elf-ppc.c
@@ -157,7 +157,7 @@ int elf_ppc_load(int argc, char **argv,	const char *buf, off_t len,
 	struct mem_ehdr ehdr;
 	char *command_line, *crash_cmdline, *cmdline_buf;
 	char *tmp_cmdline;
-	int command_line_len;
+	int command_line_len, crash_cmdline_len;
 	char *dtb;
 	int result;
 	char *error_msg;
@@ -244,19 +244,10 @@ int elf_ppc_load(int argc, char **argv,	const char *buf, off_t len,
 	} else {
 		command_line = get_command_line();
 	}
-	command_line_len = strlen(command_line) + 1;
+	command_line_len = strlen(command_line);
 
 	fixup_nodes[cur_fixup] = NULL;
 
-	/* Need to append some command line parameters internally in case of
-	 * taking crash dumps.
-	 */
-	if (info->kexec_flags & KEXEC_ON_CRASH) {
-		crash_cmdline = xmalloc(COMMAND_LINE_SIZE);
-		memset((void *)crash_cmdline, 0, COMMAND_LINE_SIZE);
-	} else
-		crash_cmdline = NULL;
-
 	/* Parse the Elf file */
 	result = build_elf_exec_info(buf, len, &ehdr, 0);
 	if (result < 0) {
@@ -292,16 +283,23 @@ int elf_ppc_load(int argc, char **argv,	const char *buf, off_t len,
 		goto out;
 	}
 
-	/* If panic kernel is being loaded, additional segments need
-	 * to be created.
+	/*
+	 * Need to append some command line parameters internally in case of
+	 * taking crash dumps. Additional segments need to be created.
 	 */
 	if (info->kexec_flags & KEXEC_ON_CRASH) {
+		crash_cmdline = xmalloc(COMMAND_LINE_SIZE);
+		memset((void *)crash_cmdline, 0, COMMAND_LINE_SIZE);
 		result = load_crashdump_segments(info, crash_cmdline,
 						max_addr, 0);
 		if (result < 0) {
 			result = -1;
 			goto out;
 		}
+		crash_cmdline_len = strlen(crash_cmdline);
+	} else {
+		crash_cmdline = NULL;
+		crash_cmdline_len = 0;
 	}
 
 	/*
@@ -337,6 +335,11 @@ int elf_ppc_load(int argc, char **argv,	const char *buf, off_t len,
 
 	info->entry = (void *)arg_base;
 #else
+	if (crash_cmdline_len + command_line_len + 1 > COMMAND_LINE_SIZE) {
+		printf("Kernel command line exceeds size\n");
+		return -1;
+	}
+
 	cmdline_buf = xmalloc(COMMAND_LINE_SIZE);
 	memset((void *)cmdline_buf, 0, COMMAND_LINE_SIZE);
 	if (command_line)
diff --git a/kexec/arch/ppc/kexec-uImage-ppc.c b/kexec/arch/ppc/kexec-uImage-ppc.c
index 9113fbe..008463b 100644
--- a/kexec/arch/ppc/kexec-uImage-ppc.c
+++ b/kexec/arch/ppc/kexec-uImage-ppc.c
@@ -82,7 +82,7 @@ static int ppc_load_bare_bits(int argc, char **argv, const char *buf,
 {
 	char *command_line, *cmdline_buf, *crash_cmdline;
 	char *tmp_cmdline;
-	int command_line_len;
+	int command_line_len, crash_cmdline_len;
 	char *dtb;
 	unsigned int addr;
 	unsigned long dtb_addr;
@@ -178,29 +178,34 @@ static int ppc_load_bare_bits(int argc, char **argv, const char *buf,
 
 	add_segment(info, buf, len, load_addr, len + _1MiB);
 
+
 	if (info->kexec_flags & KEXEC_ON_CRASH) {
                 crash_cmdline = xmalloc(COMMAND_LINE_SIZE);
                 memset((void *)crash_cmdline, 0, COMMAND_LINE_SIZE);
-        } else
-                crash_cmdline = NULL;
-
-	if (info->kexec_flags & KEXEC_ON_CRASH) {
 		ret = load_crashdump_segments(info, crash_cmdline,
 						max_addr, 0);
 		if (ret < 0) {
 			ret = -1;
 			goto out;
 		}
+		crash_cmdline_len = strlen(crash_cmdline);
+	} else {
+		crash_cmdline = NULL;
+		crash_cmdline_len = 0;
+	}
+
+	if (crash_cmdline_len + command_line_len + 1 > COMMAND_LINE_SIZE) {
+		printf("Kernel command line exceeds maximum possible length\n");
+		return -1;
 	}
 
 	cmdline_buf = xmalloc(COMMAND_LINE_SIZE);
 	memset((void *)cmdline_buf, 0, COMMAND_LINE_SIZE);
+
 	if (command_line)
-		strncat(cmdline_buf, command_line, command_line_len);
+		strcpy(cmdline_buf, command_line);
 	if (crash_cmdline)
-		strncat(cmdline_buf, crash_cmdline,
-			sizeof(crash_cmdline) -
-			strlen(crash_cmdline) - 1);
+		strncat(cmdline_buf, crash_cmdline, crash_cmdline_len);
 
 	elf_rel_build_load(info, &info->rhdr, (const char *)purgatory,
 				purgatory_size, 0, -1, -1, 0);


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [UPDATED PATCH] kexec/powerpc: Handle buffer overflow in kernel command line
  2013-04-02 12:13   ` [UPDATED PATCH] " Suzuki K. Poulose
@ 2013-04-16  6:17     ` Suzuki K. Poulose
  2013-04-16  6:37       ` Simon Horman
  0 siblings, 1 reply; 5+ messages in thread
From: Suzuki K. Poulose @ 2013-04-16  6:17 UTC (permalink / raw)
  To: horms; +Cc: kexec, zhangyanfei.yes

On 04/02/2013 05:43 PM, Suzuki K. Poulose wrote:
> Zhang,
>
> Sorry, it was my mistake. I have updated my patch based on the upstream
> tree now.
>
> ---
>
> From: Suzuki K. Poulose <suzuki@in.ibm.com>
>
> Enforce size check for kernel command line to make sure it
> doesn't overflow COMMAND_LINE_SIZE.
>
> Reported-by: Nathan D. Miller <nathanm2@us.ibm.com>
> Signed-off-by: Suzuki K. Poulose <suzuki@in.ibm.com>

Simon,

Could you please apply this one ?

Thanks
Suzuki
> ---
>   kexec/arch/ppc/kexec-elf-ppc.c    |   29 ++++++++++++++++-------------
>   kexec/arch/ppc/kexec-uImage-ppc.c |   23 ++++++++++++++---------
>   2 files changed, 30 insertions(+), 22 deletions(-)
>
> diff --git a/kexec/arch/ppc/kexec-elf-ppc.c b/kexec/arch/ppc/kexec-elf-ppc.c
> index 3daca2d..98cae9c 100644
> --- a/kexec/arch/ppc/kexec-elf-ppc.c
> +++ b/kexec/arch/ppc/kexec-elf-ppc.c
> @@ -157,7 +157,7 @@ int elf_ppc_load(int argc, char **argv,	const char *buf, off_t len,
>   	struct mem_ehdr ehdr;
>   	char *command_line, *crash_cmdline, *cmdline_buf;
>   	char *tmp_cmdline;
> -	int command_line_len;
> +	int command_line_len, crash_cmdline_len;
>   	char *dtb;
>   	int result;
>   	char *error_msg;
> @@ -244,19 +244,10 @@ int elf_ppc_load(int argc, char **argv,	const char *buf, off_t len,
>   	} else {
>   		command_line = get_command_line();
>   	}
> -	command_line_len = strlen(command_line) + 1;
> +	command_line_len = strlen(command_line);
>
>   	fixup_nodes[cur_fixup] = NULL;
>
> -	/* Need to append some command line parameters internally in case of
> -	 * taking crash dumps.
> -	 */
> -	if (info->kexec_flags & KEXEC_ON_CRASH) {
> -		crash_cmdline = xmalloc(COMMAND_LINE_SIZE);
> -		memset((void *)crash_cmdline, 0, COMMAND_LINE_SIZE);
> -	} else
> -		crash_cmdline = NULL;
> -
>   	/* Parse the Elf file */
>   	result = build_elf_exec_info(buf, len, &ehdr, 0);
>   	if (result < 0) {
> @@ -292,16 +283,23 @@ int elf_ppc_load(int argc, char **argv,	const char *buf, off_t len,
>   		goto out;
>   	}
>
> -	/* If panic kernel is being loaded, additional segments need
> -	 * to be created.
> +	/*
> +	 * Need to append some command line parameters internally in case of
> +	 * taking crash dumps. Additional segments need to be created.
>   	 */
>   	if (info->kexec_flags & KEXEC_ON_CRASH) {
> +		crash_cmdline = xmalloc(COMMAND_LINE_SIZE);
> +		memset((void *)crash_cmdline, 0, COMMAND_LINE_SIZE);
>   		result = load_crashdump_segments(info, crash_cmdline,
>   						max_addr, 0);
>   		if (result < 0) {
>   			result = -1;
>   			goto out;
>   		}
> +		crash_cmdline_len = strlen(crash_cmdline);
> +	} else {
> +		crash_cmdline = NULL;
> +		crash_cmdline_len = 0;
>   	}
>
>   	/*
> @@ -337,6 +335,11 @@ int elf_ppc_load(int argc, char **argv,	const char *buf, off_t len,
>
>   	info->entry = (void *)arg_base;
>   #else
> +	if (crash_cmdline_len + command_line_len + 1 > COMMAND_LINE_SIZE) {
> +		printf("Kernel command line exceeds size\n");
> +		return -1;
> +	}
> +
>   	cmdline_buf = xmalloc(COMMAND_LINE_SIZE);
>   	memset((void *)cmdline_buf, 0, COMMAND_LINE_SIZE);
>   	if (command_line)
> diff --git a/kexec/arch/ppc/kexec-uImage-ppc.c b/kexec/arch/ppc/kexec-uImage-ppc.c
> index 9113fbe..008463b 100644
> --- a/kexec/arch/ppc/kexec-uImage-ppc.c
> +++ b/kexec/arch/ppc/kexec-uImage-ppc.c
> @@ -82,7 +82,7 @@ static int ppc_load_bare_bits(int argc, char **argv, const char *buf,
>   {
>   	char *command_line, *cmdline_buf, *crash_cmdline;
>   	char *tmp_cmdline;
> -	int command_line_len;
> +	int command_line_len, crash_cmdline_len;
>   	char *dtb;
>   	unsigned int addr;
>   	unsigned long dtb_addr;
> @@ -178,29 +178,34 @@ static int ppc_load_bare_bits(int argc, char **argv, const char *buf,
>
>   	add_segment(info, buf, len, load_addr, len + _1MiB);
>
> +
>   	if (info->kexec_flags & KEXEC_ON_CRASH) {
>                   crash_cmdline = xmalloc(COMMAND_LINE_SIZE);
>                   memset((void *)crash_cmdline, 0, COMMAND_LINE_SIZE);
> -        } else
> -                crash_cmdline = NULL;
> -
> -	if (info->kexec_flags & KEXEC_ON_CRASH) {
>   		ret = load_crashdump_segments(info, crash_cmdline,
>   						max_addr, 0);
>   		if (ret < 0) {
>   			ret = -1;
>   			goto out;
>   		}
> +		crash_cmdline_len = strlen(crash_cmdline);
> +	} else {
> +		crash_cmdline = NULL;
> +		crash_cmdline_len = 0;
> +	}
> +
> +	if (crash_cmdline_len + command_line_len + 1 > COMMAND_LINE_SIZE) {
> +		printf("Kernel command line exceeds maximum possible length\n");
> +		return -1;
>   	}
>
>   	cmdline_buf = xmalloc(COMMAND_LINE_SIZE);
>   	memset((void *)cmdline_buf, 0, COMMAND_LINE_SIZE);
> +
>   	if (command_line)
> -		strncat(cmdline_buf, command_line, command_line_len);
> +		strcpy(cmdline_buf, command_line);
>   	if (crash_cmdline)
> -		strncat(cmdline_buf, crash_cmdline,
> -			sizeof(crash_cmdline) -
> -			strlen(crash_cmdline) - 1);
> +		strncat(cmdline_buf, crash_cmdline, crash_cmdline_len);
>
>   	elf_rel_build_load(info, &info->rhdr, (const char *)purgatory,
>   				purgatory_size, 0, -1, -1, 0);
>
>
> _______________________________________________
> kexec mailing list
> kexec@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/kexec
>


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [UPDATED PATCH] kexec/powerpc: Handle buffer overflow in kernel command line
  2013-04-16  6:17     ` Suzuki K. Poulose
@ 2013-04-16  6:37       ` Simon Horman
  0 siblings, 0 replies; 5+ messages in thread
From: Simon Horman @ 2013-04-16  6:37 UTC (permalink / raw)
  To: Suzuki K. Poulose; +Cc: kexec, zhangyanfei.yes

On Tue, Apr 16, 2013 at 11:47:34AM +0530, Suzuki K. Poulose wrote:
> On 04/02/2013 05:43 PM, Suzuki K. Poulose wrote:
> >Zhang,
> >
> >Sorry, it was my mistake. I have updated my patch based on the upstream
> >tree now.
> >
> >---
> >
> >From: Suzuki K. Poulose <suzuki@in.ibm.com>
> >
> >Enforce size check for kernel command line to make sure it
> >doesn't overflow COMMAND_LINE_SIZE.
> >
> >Reported-by: Nathan D. Miller <nathanm2@us.ibm.com>
> >Signed-off-by: Suzuki K. Poulose <suzuki@in.ibm.com>
> 
> Simon,
> 
> Could you please apply this one ?

Sorry for the delay, I have applied it now.

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2013-04-16  6:37 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-04-02  8:03 [PATCH] kexec/powerpc: Handle buffer overflow in kernel command line Suzuki K. Poulose
2013-04-02 10:58 ` Zhang Yanfei
2013-04-02 12:13   ` [UPDATED PATCH] " Suzuki K. Poulose
2013-04-16  6:17     ` Suzuki K. Poulose
2013-04-16  6:37       ` Simon Horman

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox