linux-kernel - Re: [PATCH v3 11/12] x86, boot: add fields to support load bzImage and ramdisk high

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Wed, 21 Nov 2012 11:18:50 -0800
From:	"H. Peter Anvin" <hpa@...or.com>
To:	Yinghai Lu <yinghai@...nel.org>
CC:	Thomas Gleixner <tglx@...utronix.de>, Ingo Molnar <mingo@...e.hu>,
	"Eric W. Biederman" <ebiederm@...ssion.com>,
	linux-kernel@...r.kernel.org, Rob Landley <rob@...dley.net>,
	Matt Fleming <matt.fleming@...el.com>
Subject: Re: [PATCH v3 11/12] x86, boot: add fields to support load bzImage
 and ramdisk high

On 11/21/2012 10:59 AM, Yinghai Lu wrote:
> 
> in boot_param:
> 
>         struct setup_header hdr;    /* setup header */  /* 0x1f1 */
>         __u8  _pad7[0x290-0x1f1-sizeof(struct setup_header)];
>         __u32 edd_mbr_sig_buffer[EDD_MBR_SIG_MAX];      /* 0x290 */
>         struct e820entry e820_map[E820MAX];             /* 0x2d0 */
>         __u8  _pad8[48];                                /* 0xcd0 */
>         struct edd_info eddbuf[EDDMAXNR];               /* 0xd00 */
>         __u8  _pad9[276];                               /* 0xeec */
> 
> so we can use till 0x290.
> 
> and after those three dword, will still have 7 left.
> 

Not quite... the length of the initialized header is given by the byte
at 0x201, which can be at most 0x7f unfortunately.  This means 0x280 is
the endpoint, not 0x290.  Some bootloaders rely on this.

However, from the point of view of the 32- and 64-bit entry points, this
is effectively a .data segment, but these can go into the corresponding
.bss segment, which is the rest of struct boot_params.

>>
>>> diff --git a/arch/x86/boot/compressed/cmdline.c
>>> b/arch/x86/boot/compressed/cmdline.c
>>> index b4c913c..00678d3 100644
>>> --- a/arch/x86/boot/compressed/cmdline.c
>>> +++ b/arch/x86/boot/compressed/cmdline.c
>>> @@ -17,6 +17,9 @@ static unsigned long get_cmd_line_ptr(void)
>>>   {
>>>         unsigned long cmd_line_ptr = real_mode->hdr.cmd_line_ptr;
>>>
>>> +       if (real_mode->hdr.version >= 0x020c)
>>> +               cmd_line_ptr |= (u64)real_mode->hdr.ext_cmd_line_ptr <<
>>> 32;
>>> +
>>>         return cmd_line_ptr;
>>>   }
>>
>>
>> No.  hdr.version is information from the kernel to the bootloader; it is
>> meaningless to look at it inside the kernel.
>>
> could remove them, but how about vmlinux elf.
> 
> when kexec vmlinux elf, it will fake one hdr, and fill version there.
> 
>> Same in a bunch of other places.

Then whatever loads vmlinux.elf is responsible for initializing those
fields to zero anyway.  It is still an atrocious abuse.  What we
probably need to do is to include the initialized header in a section in
vmlinux.elf containing the default struct boot_params.  This is the kind
of things that happen when people do things without thinking through all
the consequences.

	-hpa


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/