Re: [PATCH] i386:kexec-bzImage: Use "\0" as command line instead of empty command line

[Zhang Yanfei <zhangyanfei@cn.fujitsu.com>]

于 2013年04月08日 11:53, Wang YanQing 写道:
> On Mon, Apr 08, 2013 at 11:35:25AM +0800, Zhang Yanfei wrote:
>> I tried 3.8.0 kernel. Unfortunately, panicked again. For some reason, I didn't
>> see the panic message.
>>
>>> but if my memory don't lie me,
>>> I can boot v2.6.32 without a root= parameter, we had use v2.6.32 as product kernel
>>> still more than one year two years ago.
>>
>> Sigh, I tried in a real box and a kvm machine. Both panicked with no root= argument
>> message. I don't know why.
> 
> It maybe your CONFIG relation problem, I guess. I have booted ok more than 3 times.
> Are your have CONFIG_BLK_DEV_INITRD=y in .config? And your cpio format initramfs
> has init script in root directory? And your init script will auto-mount your really
> device right before switch_root into it?
> 
>> Anyway, Just from the code, your patch didn't fix all the possible place.
>> do_bzImage64_load may also call setup_linux_bootloader_parameters_high with
>> a null commandline. So why not change the check in
>> setup_linux_bootloader_parameters_high.
> Your are right. But your patch is wrong too.
> See below.
> 
>>
>> --------------------------
>> diff --git a/kexec/arch/i386/x86-linux-setup.c b/kexec/arch/i386/x86-linux-setup.c
>> index 454fad6..6eb2e6e 100644
>> --- a/kexec/arch/i386/x86-linux-setup.c
>> +++ b/kexec/arch/i386/x86-linux-setup.c
>> @@ -116,7 +116,8 @@ void setup_linux_bootloader_parameters_high(
>>         /* Fill in the command line */
>>         if (cmdline_len > COMMAND_LINE_SIZE) {
>>                 cmdline_len = COMMAND_LINE_SIZE;
>> -       }
>> +       } else if (cmdline_len == 0)
>> +               return;
> Can't just return, we must set the string termination guard like below:

I think this is ok for we have filled all the real_mode buffer with 0.

But I just found this fix still has problem...

Let's go through the code, take do_bzImage_load for example.

234         setup_size = kern16_size_needed + command_line_len +
235                          PURGATORY_CMDLINE_SIZE;
236         real_mode = xmalloc(setup_size);
237         memset(real_mode, 0, setup_size);
......
300         setup_linux_bootloader_parameters(info, real_mode, setup_base,
301                 kern16_size_needed, command_line, command_line_len,
302                 initrd, initrd_len);
......
369         cmdline_end = setup_base + kern16_size_needed + command_line_len - 1;
370         elf_rel_set_symbol(&info->rhdr, "cmdline_end", &cmdline_end,
371                            sizeof(unsigned long));

if no commandline and we set command_line_len to 0, line 369 is still wrong. It
will corrupt the kernel16 buf.

So your original patch is ok if we also set command_line_len to 1 and make
cmdline = "\0" in bzImage64_load.

> 
> + else if (cmdline_len == 0)
> +       cmdline_len = 1;
> 
> If you agreed, maybe I can resend the v2 patch.
>>         cmdline_ptr = ((char *)real_mode) + cmdline_offset;
>>         memcpy(cmdline_ptr, cmdline, cmdline_len);
>>         cmdline_ptr[cmdline_len - 1] = '\0';
>>
> 
> Thanks.
> 
> _______________________________________________
> 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