Date:	Tue, 15 Jan 2013 00:11:10 +0100
From:	Borislav Petkov <bp@...en8.de>
To:	Yinghai Lu <yinghai@...nel.org>
Cc:	"H. Peter Anvin" <hpa@...or.com>,
	Thomas Gleixner <tglx@...utronix.de>,
	Ingo Molnar <mingo@...e.hu>,
	"Eric W. Biederman" <ebiederm@...ssion.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Jan Kiszka <jan.kiszka@....de>,
	Jason Wessel <jason.wessel@...driver.com>,
	linux-kernel@...r.kernel.org, Rob Landley <rob@...dley.net>,
	Matt Fleming <matt.fleming@...el.com>,
	Gokul Caushik <caushik1@...il.com>,
	Josh Triplett <josh@...htriplett.org>,
	Joe Millenbach <jmillenbach@...il.com>
Subject: Re: [PATCH v7u1 22/31] x86, boot: add fields to support load bzImage
 and ramdisk above 4G

On Mon, Jan 14, 2013 at 02:38:36PM -0800, Yinghai Lu wrote:
> On Mon, Jan 14, 2013 at 12:26 PM, Borislav Petkov <bp@...en8.de> wrote:
> > On Mon, Jan 14, 2013 at 12:14:18PM -0800, Yinghai Lu wrote:
> >> no, no, no.
> >>
> >> bootloader does not need to know sentinel, and they only need to do:
> >>     clearing boot_param buffer and copying setup_header only
> >>
> >> even new bootloader is not supposed to know sentinel ...
> >
> > Ah, ok. I thought something was fishy because if bootloaders would know
> > about it, they'd copy setup_header and zero out the sentinel only, to
> > force the kernel to use crappy ext_* etc. values.
> >
> > How about this:
> >
> > "The sentinel variable is set by the linker script to 0xff. It is
> > supposed to be used for catching bootloaders which just copy the
> > setup_header portion and don't clear the whole boot_params buffer as
> > they are supposed to. Such bootloaders will leave the sentinel to its
> > initial value of 0xff and in this case, the kernel will assume that some
> > fields in boot_params have invalid values and zero them out."
> >
> still not  clear ...
> 
> in the kernel image, we only have setup_header, and it is around with
> other codes.
> 
> bootloader could prepare boot_param several ways.
> 1.  allocate boot_param, and memset it to 0, and copy setup_header from kernel
> image to the middle boot_param.
> 2. allocate buffer equal to whole setup section include code and setup_header.
> and copy whole setup section to buffer, and use that buffer as boot_params.
> a. use setup_code to do real mode booting. or
> b. clear used fields or store value in boot_param to use 32bit entry
> or 64bit entry.
> 
> 
> so now hope you understand my changes.
> 
> >> +       /*
> >> +        * kernel have sentinel to set as 0xff in setup link scripts,
> >> +        * so if bootloader just copy whole page from kernel image to
> >> +        * get setup_header instead of clearing boot_param buffer and
> >> +        * copying setup_header only, will leave sentinel as 0xff.
> >> +        * With that, we can tell some fields in boot_param have
> >> +        * invalid values, and we need to zero them in kernel.

Ok, but this needlessly mentiones some sort of allocation technique
which the bootloader does and which we don't care about. What we do care
about is the sentinel variable and what it means: if the bootloader
copies it accidentally, we use *that* as a trigger. So let's revise it:

"The sentinel variable is set by the linker script to 0xff. A bootloader
is supposed to only take setup_header and put it into a clean
boot_params buffer. If it turns out that it is clumsy or too generous
with the buffer, it most probably will pick up the sentinel variable
too. The fact that this variable then is still non-zero signals to
us that that we should zero out certain portions of boot_params (see
sanitize_real_mode()) because we assume that they contain garbage."

I think this is as clear as it gets.

-- 
Regards/Gruss,
    Boris.

Sent from a fat crate under my desk. Formatting is fine.
--
--
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/

Powered by blists - more mailing lists