public inbox for kexec@lists.infradead.org
 help / color / mirror / Atom feed
* [PATCH v2]kexec:i386:kexec-bzImage: Fix pass garbage command line to new kernel
@ 2013-04-08  9:43 Wang YanQing
  2013-04-08  9:51 ` Zhang Yanfei
  0 siblings, 1 reply; 4+ messages in thread
From: Wang YanQing @ 2013-04-08  9:43 UTC (permalink / raw)
  To: kexec; +Cc: tjd21, horms, zhangyanfei, ebiederm, hari, zhangyanfei.yes

I get garbage output of /proc/cmdline and in dmesg in new
kernel context when I use kexec to load new kernel bzImage
without append command line like below:

kexec -l bzImage --initrd=/boot/initramfs
kexec -e

The reason is kernel copy the command line
from the bootloader like below which copy/paste
from linux/arch/x86/kernel/head_32.S:

        movl pa(boot_params) + NEW_CL_POINTER,%esi
        andl %esi,%esi
        jz 1f                   # No command line
        movl $pa(boot_command_line),%edi
        movl $(COMMAND_LINE_SIZE/4),%ecx

This patch fix it.

[
although another patch
"kexec:i386/kexec-[bzImage|elf-x86]:x86_64/kexec-bzImage64: Use "\0" as command line instead of empty command line"
has resolved the garbage output by put a '\0' at the start, but I thinks this patch has sense too, it stop
kernel copy not command line data into the command line buffer in kernel
]

Signed-off-by: Wang YanQing <udknight@gmail.com>
---
Changes v1-v2:
1:Fix the wrong cmdline_end's value
 kexec/arch/i386/kexec-bzImage.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/kexec/arch/i386/kexec-bzImage.c b/kexec/arch/i386/kexec-bzImage.c
index 29f280d..2954f92 100644
--- a/kexec/arch/i386/kexec-bzImage.c
+++ b/kexec/arch/i386/kexec-bzImage.c
@@ -119,6 +119,7 @@ int do_bzImage_load(struct kexec_info *info,
 	unsigned long cmdline_end;
 	unsigned long kern16_size_needed;
 	unsigned long heap_size = 0;
+	off_t alloc_command_line_len = 0;
 
 	/*
 	 * Find out about the file I am about to load.
@@ -145,10 +146,12 @@ int do_bzImage_load(struct kexec_info *info,
 			dbgprintf("Kernel command line too long for kernel!\n");
 			return -1;
 		}
+		alloc_command_line_len = (uintmax_t)setup_header.cmdline_size;
 	} else {
 		if (command_line_len > 255) {
 			dbgprintf("WARNING: This kernel may only support 255 byte command lines\n");
 		}
+		alloc_command_line_len = 255;
 	}
 
 	if (setup_header.protocol_version >= 0x0205) {
@@ -231,7 +234,7 @@ int do_bzImage_load(struct kexec_info *info,
 		if (kern16_size_needed < 4096)
 			kern16_size_needed = 4096;
 	}
-	setup_size = kern16_size_needed + command_line_len +
+	setup_size = kern16_size_needed + alloc_command_line_len +
 			 PURGATORY_CMDLINE_SIZE;
 	real_mode = xmalloc(setup_size);
 	memset(real_mode, 0, setup_size);
@@ -366,7 +369,7 @@ int do_bzImage_load(struct kexec_info *info,
 					 &regs16, sizeof(regs16));
 	}
 	elf_rel_set_symbol(&info->rhdr, "entry32_regs", &regs32, sizeof(regs32));
-	cmdline_end = setup_base + kern16_size_needed + command_line_len - 1;
+	cmdline_end = setup_base + kern16_size_needed + alloc_command_line_len - 1;
 	elf_rel_set_symbol(&info->rhdr, "cmdline_end", &cmdline_end,
 			   sizeof(unsigned long));
 
-- 
1.7.12.4.dirty

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

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

* Re: [PATCH v2]kexec:i386:kexec-bzImage: Fix pass garbage command line to new kernel
  2013-04-08  9:43 [PATCH v2]kexec:i386:kexec-bzImage: Fix pass garbage command line to new kernel Wang YanQing
@ 2013-04-08  9:51 ` Zhang Yanfei
  2013-04-09  0:57   ` Wang YanQing
  2013-04-09  1:08   ` Wang YanQing
  0 siblings, 2 replies; 4+ messages in thread
From: Zhang Yanfei @ 2013-04-08  9:51 UTC (permalink / raw)
  To: Wang YanQing; +Cc: tjd21, kexec, horms, ebiederm, hari, zhangyanfei.yes

于 2013年04月08日 17:43, Wang YanQing 写道:
> I get garbage output of /proc/cmdline and in dmesg in new
> kernel context when I use kexec to load new kernel bzImage
> without append command line like below:
> 
> kexec -l bzImage --initrd=/boot/initramfs
> kexec -e
> 
> The reason is kernel copy the command line
> from the bootloader like below which copy/paste
> from linux/arch/x86/kernel/head_32.S:
> 
>         movl pa(boot_params) + NEW_CL_POINTER,%esi
>         andl %esi,%esi
>         jz 1f                   # No command line
>         movl $pa(boot_command_line),%edi
>         movl $(COMMAND_LINE_SIZE/4),%ecx
> 
> This patch fix it.
> 
> [
> although another patch
> "kexec:i386/kexec-[bzImage|elf-x86]:x86_64/kexec-bzImage64: Use "\0" as command line instead of empty command line"
> has resolved the garbage output by put a '\0' at the start, but I thinks this patch has sense too, it stop
> kernel copy not command line data into the command line buffer in kernel

I don't think the patch is necessary, please see below.

> ]
> 
> Signed-off-by: Wang YanQing <udknight@gmail.com>
> ---
> Changes v1-v2:
> 1:Fix the wrong cmdline_end's value
>  kexec/arch/i386/kexec-bzImage.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/kexec/arch/i386/kexec-bzImage.c b/kexec/arch/i386/kexec-bzImage.c
> index 29f280d..2954f92 100644
> --- a/kexec/arch/i386/kexec-bzImage.c
> +++ b/kexec/arch/i386/kexec-bzImage.c
> @@ -119,6 +119,7 @@ int do_bzImage_load(struct kexec_info *info,
>  	unsigned long cmdline_end;
>  	unsigned long kern16_size_needed;
>  	unsigned long heap_size = 0;
> +	off_t alloc_command_line_len = 0;
>  
>  	/*
>  	 * Find out about the file I am about to load.
> @@ -145,10 +146,12 @@ int do_bzImage_load(struct kexec_info *info,
>  			dbgprintf("Kernel command line too long for kernel!\n");
>  			return -1;
>  		}
> +		alloc_command_line_len = (uintmax_t)setup_header.cmdline_size;
>  	} else {
>  		if (command_line_len > 255) {
>  			dbgprintf("WARNING: This kernel may only support 255 byte command lines\n");
>  		}
> +		alloc_command_line_len = 255;
>  	}
>  
>  	if (setup_header.protocol_version >= 0x0205) {
> @@ -231,7 +234,7 @@ int do_bzImage_load(struct kexec_info *info,
>  		if (kern16_size_needed < 4096)
>  			kern16_size_needed = 4096;
>  	}
> -	setup_size = kern16_size_needed + command_line_len +
> +	setup_size = kern16_size_needed + alloc_command_line_len +
>  			 PURGATORY_CMDLINE_SIZE;
>  	real_mode = xmalloc(setup_size);
>  	memset(real_mode, 0, setup_size);
> @@ -366,7 +369,7 @@ int do_bzImage_load(struct kexec_info *info,
>  					 &regs16, sizeof(regs16));
>  	}
>  	elf_rel_set_symbol(&info->rhdr, "entry32_regs", &regs32, sizeof(regs32));
> -	cmdline_end = setup_base + kern16_size_needed + command_line_len - 1;
> +	cmdline_end = setup_base + kern16_size_needed + alloc_command_line_len - 1;

This is obviously wrong. 
Purgatory may append a kexec_jump_back_entry= argument into the commandline. It is
an argument belongs to the commandline. So why there is a buf filled with 0 
(alloc_command_line_len) before it in the commandline buf.

>  	elf_rel_set_symbol(&info->rhdr, "cmdline_end", &cmdline_end,
>  			   sizeof(unsigned long));
>  


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

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

* Re: [PATCH v2]kexec:i386:kexec-bzImage: Fix pass garbage command line to new kernel
  2013-04-08  9:51 ` Zhang Yanfei
@ 2013-04-09  0:57   ` Wang YanQing
  2013-04-09  1:08   ` Wang YanQing
  1 sibling, 0 replies; 4+ messages in thread
From: Wang YanQing @ 2013-04-09  0:57 UTC (permalink / raw)
  To: Zhang Yanfei; +Cc: tjd21, kexec, horms, ebiederm, hari, zhangyanfei.yes

On Mon, Apr 08, 2013 at 05:51:52PM +0800, Zhang Yanfei wrote:
> I don't think the patch is necessary, please see below.

I think bootloader prepare garbage in the command line
buffer which will be passed  into kernel command line buffer 
is not good, and I should use RFC instead of PATCH in 
the subject, I haven't test this patch before send it out, sorry.

After all, I have no time to dig this problem deeper right now,
I will prepare another patch to fix it if I have time to understand 
all the relative codes about this problem.

Thanks.

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

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

* Re: [PATCH v2]kexec:i386:kexec-bzImage: Fix pass garbage command line to new kernel
  2013-04-08  9:51 ` Zhang Yanfei
  2013-04-09  0:57   ` Wang YanQing
@ 2013-04-09  1:08   ` Wang YanQing
  1 sibling, 0 replies; 4+ messages in thread
From: Wang YanQing @ 2013-04-09  1:08 UTC (permalink / raw)
  To: Zhang Yanfei; +Cc: tjd21, kexec, horms, ebiederm, hari, zhangyanfei.yes

On Mon, Apr 08, 2013 at 05:51:52PM +0800, Zhang Yanfei wrote:

> This is obviously wrong. 
Yes, it is obviously wrong
> Purgatory may append a kexec_jump_back_entry= argument into the commandline. It is
> an argument belongs to the commandline. So why there is a buf filled with 0 
> (alloc_command_line_len) before it in the commandline buf.
Kernel will copy all the COMMAND_LINE_SIZE len buffer pointer by 
cmd_line_ptr, but kexec-bzImage just prepare a very shorter buffer
in the below line check failed code path:

if (info->kexec_flags & (KEXEC_ON_CRASH | KEXEC_PRESERVE_CONTEXT))

in the check sucess code path, it seems good by below line:
modified_cmdline = xmalloc(COMMAND_LINE_SIZE);


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

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

end of thread, other threads:[~2013-04-09  1:09 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-04-08  9:43 [PATCH v2]kexec:i386:kexec-bzImage: Fix pass garbage command line to new kernel Wang YanQing
2013-04-08  9:51 ` Zhang Yanfei
2013-04-09  0:57   ` Wang YanQing
2013-04-09  1:08   ` Wang YanQing

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