* [PATCH] x86, boot: access cmdline when loaded high
@ 2013-06-11 18:41 Kees Cook
2013-06-11 19:17 ` Yinghai Lu
0 siblings, 1 reply; 6+ messages in thread
From: Kees Cook @ 2013-06-11 18:41 UTC (permalink / raw)
To: linux-kernel
Cc: H. Peter Anvin, Thomas Gleixner, Ingo Molnar, x86, Yinghai Lu,
Gokul Caushik, Kees Cook, Josh Triplett
The __cmdline_find_option routine requires that the kernel cmdline be
located under 0x10000. When running the compressed boot code, if the
cmdline is loaded above this range, it will be ignored. This breaks
recognition of things like "earlyprintk=...". Since the compressed boot
code is already tricking the cmdline routines about locations, take it all
the way and access the cmdline via offset instead of via actual location.
Signed-off-by: Kees Cook <keescook@chromium.org>
---
arch/x86/boot/compressed/cmdline.c | 21 +++++++++------------
1 file changed, 9 insertions(+), 12 deletions(-)
diff --git a/arch/x86/boot/compressed/cmdline.c b/arch/x86/boot/compressed/cmdline.c
index bffd73b..2e39280 100644
--- a/arch/x86/boot/compressed/cmdline.c
+++ b/arch/x86/boot/compressed/cmdline.c
@@ -2,32 +2,29 @@
#ifdef CONFIG_EARLY_PRINTK
+/*
+ * We know where the cmdline is, so just retain it, and use a non-zero
+ * offset to trick __cmdline_find_option into running.
+ */
static unsigned long fs;
static inline void set_fs(unsigned long seg)
{
- fs = seg << 4; /* shift it back */
+ fs = real_mode->hdr.cmd_line_ptr;
+ fs |= (u64)real_mode->ext_cmd_line_ptr << 32;
}
typedef unsigned long addr_t;
static inline char rdfs8(addr_t addr)
{
- return *((char *)(fs + addr));
+ return *((char *)(fs + addr - 0x1));
}
#include "../cmdline.c"
-static unsigned long get_cmd_line_ptr(void)
-{
- unsigned long cmd_line_ptr = real_mode->hdr.cmd_line_ptr;
-
- cmd_line_ptr |= (u64)real_mode->ext_cmd_line_ptr << 32;
-
- return cmd_line_ptr;
-}
int cmdline_find_option(const char *option, char *buffer, int bufsize)
{
- return __cmdline_find_option(get_cmd_line_ptr(), option, buffer, bufsize);
+ return __cmdline_find_option(0x1, option, buffer, bufsize);
}
int cmdline_find_option_bool(const char *option)
{
- return __cmdline_find_option_bool(get_cmd_line_ptr(), option);
+ return __cmdline_find_option_bool(0x1, option);
}
#endif
--
1.7.9.5
--
Kees Cook
Chrome OS Security
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] x86, boot: access cmdline when loaded high
2013-06-11 18:41 [PATCH] x86, boot: access cmdline when loaded high Kees Cook
@ 2013-06-11 19:17 ` Yinghai Lu
2013-06-11 19:26 ` Kees Cook
0 siblings, 1 reply; 6+ messages in thread
From: Yinghai Lu @ 2013-06-11 19:17 UTC (permalink / raw)
To: Kees Cook
Cc: Linux Kernel Mailing List, H. Peter Anvin, Thomas Gleixner,
Ingo Molnar, the arch/x86 maintainers, Gokul Caushik,
Josh Triplett
On Tue, Jun 11, 2013 at 11:41 AM, Kees Cook <keescook@chromium.org> wrote:
> The __cmdline_find_option routine requires that the kernel cmdline be
> located under 0x10000. When running the compressed boot code, if the
> cmdline is loaded above this range, it will be ignored. This breaks
> recognition of things like "earlyprintk=...". Since the compressed boot
> code is already tricking the cmdline routines about locations, take it all
> the way and access the cmdline via offset instead of via actual location.
Hi,
What is point of the patch?
It looks like it try to fix one bug, but it doesn't,
as there is no bug out there.
Before the patch, command line that is loaded high
can be processed correctly in arch/x86/boot/compressed/cmdline.c.
>
> Signed-off-by: Kees Cook <keescook@chromium.org>
> ---
> arch/x86/boot/compressed/cmdline.c | 21 +++++++++------------
> 1 file changed, 9 insertions(+), 12 deletions(-)
>
> diff --git a/arch/x86/boot/compressed/cmdline.c b/arch/x86/boot/compressed/cmdline.c
> index bffd73b..2e39280 100644
> --- a/arch/x86/boot/compressed/cmdline.c
> +++ b/arch/x86/boot/compressed/cmdline.c
> @@ -2,32 +2,29 @@
>
> #ifdef CONFIG_EARLY_PRINTK
>
> +/*
> + * We know where the cmdline is, so just retain it, and use a non-zero
> + * offset to trick __cmdline_find_option into running.
> + */
> static unsigned long fs;
> static inline void set_fs(unsigned long seg)
> {
> - fs = seg << 4; /* shift it back */
> + fs = real_mode->hdr.cmd_line_ptr;
> + fs |= (u64)real_mode->ext_cmd_line_ptr << 32;
> }
> typedef unsigned long addr_t;
> static inline char rdfs8(addr_t addr)
> {
> - return *((char *)(fs + addr));
> + return *((char *)(fs + addr - 0x1));
> }
> #include "../cmdline.c"
> -static unsigned long get_cmd_line_ptr(void)
> -{
> - unsigned long cmd_line_ptr = real_mode->hdr.cmd_line_ptr;
> -
> - cmd_line_ptr |= (u64)real_mode->ext_cmd_line_ptr << 32;
> -
> - return cmd_line_ptr;
> -}
> int cmdline_find_option(const char *option, char *buffer, int bufsize)
> {
> - return __cmdline_find_option(get_cmd_line_ptr(), option, buffer, bufsize);
> + return __cmdline_find_option(0x1, option, buffer, bufsize);
that's funny, why do you pick 0x1 here?
why not use 0x10 or 0?
> }
> int cmdline_find_option_bool(const char *option)
> {
> - return __cmdline_find_option_bool(get_cmd_line_ptr(), option);
> + return __cmdline_find_option_bool(0x1, option);
> }
>
> #endif
> --
> 1.7.9.5
>
>
> --
> Kees Cook
> Chrome OS Security
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] x86, boot: access cmdline when loaded high
2013-06-11 19:17 ` Yinghai Lu
@ 2013-06-11 19:26 ` Kees Cook
2013-06-11 20:54 ` Yinghai Lu
0 siblings, 1 reply; 6+ messages in thread
From: Kees Cook @ 2013-06-11 19:26 UTC (permalink / raw)
To: Yinghai Lu
Cc: Linux Kernel Mailing List, H. Peter Anvin, Thomas Gleixner,
Ingo Molnar, the arch/x86 maintainers, Gokul Caushik,
Josh Triplett
On Tue, Jun 11, 2013 at 12:17 PM, Yinghai Lu <yinghai@kernel.org> wrote:
> On Tue, Jun 11, 2013 at 11:41 AM, Kees Cook <keescook@chromium.org> wrote:
>> The __cmdline_find_option routine requires that the kernel cmdline be
>> located under 0x10000. When running the compressed boot code, if the
>> cmdline is loaded above this range, it will be ignored. This breaks
>> recognition of things like "earlyprintk=...". Since the compressed boot
>> code is already tricking the cmdline routines about locations, take it all
>> the way and access the cmdline via offset instead of via actual location.
>
> Hi,
>
> What is point of the patch?
> It looks like it try to fix one bug, but it doesn't,
> as there is no bug out there.
>
> Before the patch, command line that is loaded high
> can be processed correctly in arch/x86/boot/compressed/cmdline.c.
It can't. Here is the code from arch/x86/boot/cmdline.c:
int __cmdline_find_option(unsigned long cmdline_ptr, const char
*option, char *buffer, int bufsize)
...
if (!cmdline_ptr)
return -1; /* No command line */
cptr = cmdline_ptr & 0xf;
set_fs(cmdline_ptr >> 4);
while (cptr < 0x10000 && (c = rdfs8(cptr++))) {
This means that if get_cmd_line_ptr() returns an address >= 0x10000,
__cmdline_find_option will simply ignore the cmdline.
>> Signed-off-by: Kees Cook <keescook@chromium.org>
>> ---
>> arch/x86/boot/compressed/cmdline.c | 21 +++++++++------------
>> 1 file changed, 9 insertions(+), 12 deletions(-)
>>
>> diff --git a/arch/x86/boot/compressed/cmdline.c b/arch/x86/boot/compressed/cmdline.c
>> index bffd73b..2e39280 100644
>> --- a/arch/x86/boot/compressed/cmdline.c
>> +++ b/arch/x86/boot/compressed/cmdline.c
>> @@ -2,32 +2,29 @@
>>
>> #ifdef CONFIG_EARLY_PRINTK
>>
>> +/*
>> + * We know where the cmdline is, so just retain it, and use a non-zero
>> + * offset to trick __cmdline_find_option into running.
>> + */
>> static unsigned long fs;
>> static inline void set_fs(unsigned long seg)
>> {
>> - fs = seg << 4; /* shift it back */
>> + fs = real_mode->hdr.cmd_line_ptr;
>> + fs |= (u64)real_mode->ext_cmd_line_ptr << 32;
>> }
>> typedef unsigned long addr_t;
>> static inline char rdfs8(addr_t addr)
>> {
>> - return *((char *)(fs + addr));
>> + return *((char *)(fs + addr - 0x1));
>> }
>> #include "../cmdline.c"
>> -static unsigned long get_cmd_line_ptr(void)
>> -{
>> - unsigned long cmd_line_ptr = real_mode->hdr.cmd_line_ptr;
>> -
>> - cmd_line_ptr |= (u64)real_mode->ext_cmd_line_ptr << 32;
>> -
>> - return cmd_line_ptr;
>> -}
>> int cmdline_find_option(const char *option, char *buffer, int bufsize)
>> {
>> - return __cmdline_find_option(get_cmd_line_ptr(), option, buffer, bufsize);
>> + return __cmdline_find_option(0x1, option, buffer, bufsize);
>
> that's funny, why do you pick 0x1 here?
> why not use 0x10 or 0?
0 cannot be used because of the !cmdline_ptr test in
__cmdline_find_option. There is no reason it's 1, except that it's the
further possible from 0x10000 without being 0.
-Kees
>
>> }
>> int cmdline_find_option_bool(const char *option)
>> {
>> - return __cmdline_find_option_bool(get_cmd_line_ptr(), option);
>> + return __cmdline_find_option_bool(0x1, option);
>> }
>>
>> #endif
>> --
>> 1.7.9.5
>>
>>
>> --
>> Kees Cook
>> Chrome OS Security
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>> Please read the FAQ at http://www.tux.org/lkml/
--
Kees Cook
Chrome OS Security
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] x86, boot: access cmdline when loaded high
2013-06-11 19:26 ` Kees Cook
@ 2013-06-11 20:54 ` Yinghai Lu
2013-06-11 20:59 ` Yinghai Lu
2013-06-13 0:05 ` Kees Cook
0 siblings, 2 replies; 6+ messages in thread
From: Yinghai Lu @ 2013-06-11 20:54 UTC (permalink / raw)
To: Kees Cook
Cc: Linux Kernel Mailing List, H. Peter Anvin, Thomas Gleixner,
Ingo Molnar, the arch/x86 maintainers, Gokul Caushik,
Josh Triplett
On Tue, Jun 11, 2013 at 12:26 PM, Kees Cook <keescook@chromium.org> wrote:
> On Tue, Jun 11, 2013 at 12:17 PM, Yinghai Lu <yinghai@kernel.org> wrote:
>> On Tue, Jun 11, 2013 at 11:41 AM, Kees Cook <keescook@chromium.org> wrote:
>>> The __cmdline_find_option routine requires that the kernel cmdline be
>>> located under 0x10000. When running the compressed boot code, if the
>>> cmdline is loaded above this range, it will be ignored. This breaks
>>> recognition of things like "earlyprintk=...". Since the compressed boot
>>> code is already tricking the cmdline routines about locations, take it all
>>> the way and access the cmdline via offset instead of via actual location.
>>
>> Hi,
>>
>> What is point of the patch?
>> It looks like it try to fix one bug, but it doesn't,
>> as there is no bug out there.
>>
>> Before the patch, command line that is loaded high
>> can be processed correctly in arch/x86/boot/compressed/cmdline.c.
>
> It can't. Here is the code from arch/x86/boot/cmdline.c:
Are you sure?
I tested that with kexec loaded second kernel high.
How do you test that?
>
> int __cmdline_find_option(unsigned long cmdline_ptr, const char
> *option, char *buffer, int bufsize)
> ...
> if (!cmdline_ptr)
> return -1; /* No command line */
>
> cptr = cmdline_ptr & 0xf;
> set_fs(cmdline_ptr >> 4);
>
> while (cptr < 0x10000 && (c = rdfs8(cptr++))) {
>
>
> This means that if get_cmd_line_ptr() returns an address >= 0x10000,
> __cmdline_find_option will simply ignore the cmdline.
No, you read the code wrong. you miss the
cptr = cmdline_ptr & 0xf;
Yinghai
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] x86, boot: access cmdline when loaded high
2013-06-11 20:54 ` Yinghai Lu
@ 2013-06-11 20:59 ` Yinghai Lu
2013-06-13 0:05 ` Kees Cook
1 sibling, 0 replies; 6+ messages in thread
From: Yinghai Lu @ 2013-06-11 20:59 UTC (permalink / raw)
To: Kees Cook
Cc: Linux Kernel Mailing List, H. Peter Anvin, Thomas Gleixner,
Ingo Molnar, the arch/x86 maintainers, Gokul Caushik,
Josh Triplett
On Tue, Jun 11, 2013 at 1:54 PM, Yinghai Lu <yinghai@kernel.org> wrote:
>>
>>> Before the patch, command line that is loaded high
>>> can be processed correctly in arch/x86/boot/compressed/cmdline.c.
>>
>> It can't. Here is the code from arch/x86/boot/cmdline.c:
>
> Are you sure?
>
> I tested that with kexec loaded second kernel high.
>
> How do you test that?
double checked, it works already:
10:~/k # sh kk
add_buffer: base:11fff6000 bufsz:80e0 memsz:a000
add_buffer: base:11fff0000 bufsz:44ce memsz:44ce
add_buffer: base:11d000000 bufsz:9084f0 memsz:2c43000
add_buffer: base:11acef000 bufsz:230f6ec memsz:230f6ec
[ 33.551046] cpu0 machine_kexec: fn: ffffffff81076960, data: ffff880116a20000
10:~/k # kexec -d -e
[ 52.939049] psmouse serio1: shutdown
...
[ 52.965553] Starting new kernel
early console in decompress_kernel ============> from misc.c
decompress_kernel:
input: [0x11ea902c2-0x11f390ba1], output: 0x11d000000, heap:
[0x11f398500-0x11f3a04ff] ============>
from misc.c
Decompressing Linux... xz... Parsing ELF... done. ========> from misc.c
Booting the kernel.
[ 0.000000] real_mode_data : phys 000000011fff0000
[ 0.000000] real_mode_data : virt ffff88011fff0000
[ 0.000000] boot_params : init virt ffffffff82f839a0
[ 0.000000] boot_params : phys 000000011ef839a0
[ 0.000000] boot_params : virt ffff88011ef839a0
[ 0.000000] boot_command_line : init virt ffffffff82e51020
[ 0.000000] boot_command_line : phys 000000011ee51020 ===> is high ?
[ 0.000000] boot_command_line : virt ffff88011ee51020
[ 0.000000] Kernel Layout:
[ 0.000000] .text: [0x11d000000-0x11e0ae480]
[ 0.000000] .rodata: [0x11e200000-0x11e9cefff]
[ 0.000000] .data: [0x11ea00000-0x11ebd3fbf]
[ 0.000000] .init: [0x11ebd5000-0x11ef6efff]
[ 0.000000] .bss: [0x11ef7d000-0x11fc1dfff]
[ 0.000000] .brk: [0x11fc1e000-0x11fc42fff]
[ 0.000000] memblock_reserve: [0x0009f000-0x000fffff] * BIOS reserved
[ 0.000000] Initializing cgroup subsys cpuset
[ 0.000000] Initializing cgroup subsys cpu
[ 0.000000] Initializing cgroup subsys cpuacct
[ 0.000000] Linux version 3.10.0-rc5-yh-00864-g9cca511-dirty
(yhlu@linux-siqj.site) (gcc version 4.7.2 20130108 [gcc-4_7-branch
revision 195012] (SUSE Linux) ) #1732 SMP Tue Jun 11 13:47:35 PDT 2013
[ 0.000000] memblock_reserve: [0x11d000000-0x11fc1dfff] TEXT DATA BSS
[ 0.000000] memblock_reserve: [0x11acef000-0x11cffefff] RAMDISK
[ 0.000000] Command line: initcall_debug pci=routeirq debug
ignore_loglevel apic=debug ramdisk_size=262144 root=/dev/ram0 rw
ip=dhcp console=uart8250,io,0x3f8,115200n8
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] x86, boot: access cmdline when loaded high
2013-06-11 20:54 ` Yinghai Lu
2013-06-11 20:59 ` Yinghai Lu
@ 2013-06-13 0:05 ` Kees Cook
1 sibling, 0 replies; 6+ messages in thread
From: Kees Cook @ 2013-06-13 0:05 UTC (permalink / raw)
To: Yinghai Lu
Cc: Linux Kernel Mailing List, H. Peter Anvin, Thomas Gleixner,
Ingo Molnar, the arch/x86 maintainers, Gokul Caushik,
Josh Triplett
On Tue, Jun 11, 2013 at 1:54 PM, Yinghai Lu <yinghai@kernel.org> wrote:
> On Tue, Jun 11, 2013 at 12:26 PM, Kees Cook <keescook@chromium.org> wrote:
>> On Tue, Jun 11, 2013 at 12:17 PM, Yinghai Lu <yinghai@kernel.org> wrote:
>>> On Tue, Jun 11, 2013 at 11:41 AM, Kees Cook <keescook@chromium.org> wrote:
>>>> The __cmdline_find_option routine requires that the kernel cmdline be
>>>> located under 0x10000. When running the compressed boot code, if the
>>>> cmdline is loaded above this range, it will be ignored. This breaks
>>>> recognition of things like "earlyprintk=...". Since the compressed boot
>>>> code is already tricking the cmdline routines about locations, take it all
>>>> the way and access the cmdline via offset instead of via actual location.
>>>
>>> Hi,
>>>
>>> What is point of the patch?
>>> It looks like it try to fix one bug, but it doesn't,
>>> as there is no bug out there.
>>>
>>> Before the patch, command line that is loaded high
>>> can be processed correctly in arch/x86/boot/compressed/cmdline.c.
>>
>> It can't. Here is the code from arch/x86/boot/cmdline.c:
>
> Are you sure?
>
> I tested that with kexec loaded second kernel high.
>
> How do you test that?
>
>>
>> int __cmdline_find_option(unsigned long cmdline_ptr, const char
>> *option, char *buffer, int bufsize)
>> ...
>> if (!cmdline_ptr)
>> return -1; /* No command line */
>>
>> cptr = cmdline_ptr & 0xf;
>> set_fs(cmdline_ptr >> 4);
>>
>> while (cptr < 0x10000 && (c = rdfs8(cptr++))) {
>>
>>
>> This means that if get_cmd_line_ptr() returns an address >= 0x10000,
>> __cmdline_find_option will simply ignore the cmdline.
>
> No, you read the code wrong. you miss the
>
> cptr = cmdline_ptr & 0xf;
Ah-ha! I see what I missed. I didn't notice 16a4baa6 which did this:
- if (!cmdline_ptr || cmdline_ptr >= 0x100000)
- return -1; /* No command line, or inaccessible */
+ if (!cmdline_ptr)
+ return -1; /* No command line */
That fixes the problem I was seeing. :)
Thanks!
-Kees
--
Kees Cook
Chrome OS Security
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-06-13 0:05 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-06-11 18:41 [PATCH] x86, boot: access cmdline when loaded high Kees Cook
2013-06-11 19:17 ` Yinghai Lu
2013-06-11 19:26 ` Kees Cook
2013-06-11 20:54 ` Yinghai Lu
2013-06-11 20:59 ` Yinghai Lu
2013-06-13 0:05 ` Kees Cook
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®