Re: [PATCH v3 1/7] x86, kaslr: Use init_size instead of run_size
On Sat, Mar 14, 2015 at 08:53:57AM +0100, Ingo Molnar wrote: > You are still talking only about the low level code and about low > level symptoms, while in contrast to that the primary question with... Thanks for doing that! I really doubt it'll bring anything this time either but I don't care - certainly I've wasted enough time trying to improve things as far as I can so hell yeah, this bullshit has to stop! In addition to what you've said, I have only one proposal: we revert the whole crap and do it again, this time nice and clean in tip. It'll stay there as long as it has to. No half-arsed commit messages, no misunderstood crap, no lala code f*ckery. Whatever it takes, as long as it takes. Thanks. -- Regards/Gruss, Boris. ECO tip #101: Trim your mails when you reply. -- -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3 1/7] x86, kaslr: Use init_size instead of run_size
* Yinghai Lu wrote:
> On Fri, Mar 13, 2015 at 5:27 AM, Ingo Molnar wrote:
> >
> > * Yinghai Lu wrote:
> >
> >> commit e6023367d779 ("x86, kaslr: Prevent .bss from overlaping
> >> initrd") introduced one run_size for kaslr. We should use real
> >> runtime size (include copy/decompress) aka init_size.
> >
> > Why, what happens if we don't change this?
>
> While trying to update change log, found we still need to keep this
> run_size. otherwise kaslr will find not needed big size of init_size
> instead of max(output_len, run_size).
You are still talking only about the low level code and about low
level symptoms, while in contrast to that the primary question with
_any_ change to the kernel is always:
WHY ARE WE CHANGING THE KERNEL, WHAT BAD HIGH LEVEL BEHAVIOR CAN A
USER OBSERVE IF THE BUG IS NOT FIXED?
Your descriptions totally ignore the high level effects of the bug on
the system and on users of the machine, and you fail to describe them
properly. You totally concentrate on the low level: your descriptions
are missing the forest from all the trees.
That makes 90% of your patch descriptions useless.
In fact, 90% of your patches had that deficiency and had it for the
past 4 years, non-stop, and maintainers were complaining to you about
that, non-stop as well. Do you think maintainers enjoy complaining
about deficiencies? Do you wonder why maintainers were forced to
simply stop looking at any complex series from yours after you refused
to change?
> will refresh the patchset.
So let me try this again, one very last time.
That sentence demonstrates your problem: it's not a 'refresh' your
patches need, but a 'hard reboot', a totally new viewpoint that
concentrates on what matters: that zooms out of the small details of
the patch!
For any serious change to the Linux kernel, analyzing small details is
fine and required as well, AFTER the high level has been discussed
properly:
- What happened, what high level concern motivates you to change the
Linux kernel?
And no, starting a changelog with:
commit e6023367d779 ("x86, kaslr: Prevent .bss from
overlaping initrd") introduced one run_size for kaslr.
is not 'high level' in any way, it talks about code in the
first sentence! Talking about code, not talking about high
level concerns is a BUG().
- What was the previous (often bad) high level behavior of the
kernel?
And no, 'KASLR will not find a proper area' is NOT a high level
description, it's a very low level description! Not discussing
high level behavior of the kernel in a changelog is a BUG().
- What new high level behavior do we want to happen instead?
And no, saying that 'KASLR should be passed init_size instead
of run_size' is not a description of desired new high level
behavior! Not discussing the desired high level behavior of the
kernel in a changelog is a BUG().
- What was the high level design of the old code, was that design
fine, should it be changed, and if yes, in what way?
Note that we describe the high level design even if we don't
change it, partly to give context to the reader, partly to
prove that you know what you are doing, to build trust in your
patch! Not discussing the old (and new) design of that area of
the kernel is a BUG().
and only then do we:
- Describe the old implementation, and describe how the new
implementation works in all that context.
Here is where 99.9% of your existing changelogs fit in.
Put differently: your changelogs are missing the first 3
essential components of a changelog.
And note, mentioning 'run_size' in a low level description is
fine. Mentioning it in a high level description is a BUG():
that is why Boris was trying to change your changelogs into
spoken English, to recover some of that missing high level
context. Maintainers like Boris should not be forced to do
that, you are supposed to offer this with any significant
patch, as a passport to prove that your changes to the code are
worth looking at.
And yes, we do it in that order. Take a look at a recent single line
change Linus made in 53da3bc2ba9e48, attached to this mail.
Check how the changelog is structured: it discusses high level
concepts first. It's a _ONE LINER FIX_ from Linus, yet it's spread
over 8 paragraphs and 50 lines, because the change justified that kind
of description.
And observe the moment when Linus, in his 8 paragraphs, 50 lines
description starts talking about low level implementational details,
when he mentions lines of code, function names, such as
do_numa_page(), 'pte_write()' or 'pte_dirty()'.
He doesnt!
It's not needed for a one-liner most of the time: but the high level
concepts around that code are very much necessary to convey.
Now contrast that with your changelogs:
Re: [PATCH v3 1/7] x86, kaslr: Use init_size instead of run_size
* Yinghai Lu ying...@kernel.org wrote: On Fri, Mar 13, 2015 at 5:27 AM, Ingo Molnar mi...@kernel.org wrote: * Yinghai Lu ying...@kernel.org wrote: commit e6023367d779 (x86, kaslr: Prevent .bss from overlaping initrd) introduced one run_size for kaslr. We should use real runtime size (include copy/decompress) aka init_size. Why, what happens if we don't change this? While trying to update change log, found we still need to keep this run_size. otherwise kaslr will find not needed big size of init_size instead of max(output_len, run_size). You are still talking only about the low level code and about low level symptoms, while in contrast to that the primary question with _any_ change to the kernel is always: WHY ARE WE CHANGING THE KERNEL, WHAT BAD HIGH LEVEL BEHAVIOR CAN A USER OBSERVE IF THE BUG IS NOT FIXED? Your descriptions totally ignore the high level effects of the bug on the system and on users of the machine, and you fail to describe them properly. You totally concentrate on the low level: your descriptions are missing the forest from all the trees. That makes 90% of your patch descriptions useless. In fact, 90% of your patches had that deficiency and had it for the past 4 years, non-stop, and maintainers were complaining to you about that, non-stop as well. Do you think maintainers enjoy complaining about deficiencies? Do you wonder why maintainers were forced to simply stop looking at any complex series from yours after you refused to change? will refresh the patchset. So let me try this again, one very last time. That sentence demonstrates your problem: it's not a 'refresh' your patches need, but a 'hard reboot', a totally new viewpoint that concentrates on what matters: that zooms out of the small details of the patch! For any serious change to the Linux kernel, analyzing small details is fine and required as well, AFTER the high level has been discussed properly: - What happened, what high level concern motivates you to change the Linux kernel? And no, starting a changelog with: commit e6023367d779 (x86, kaslr: Prevent .bss from overlaping initrd) introduced one run_size for kaslr. is not 'high level' in any way, it talks about code in the first sentence! Talking about code, not talking about high level concerns is a BUG(). - What was the previous (often bad) high level behavior of the kernel? And no, 'KASLR will not find a proper area' is NOT a high level description, it's a very low level description! Not discussing high level behavior of the kernel in a changelog is a BUG(). - What new high level behavior do we want to happen instead? And no, saying that 'KASLR should be passed init_size instead of run_size' is not a description of desired new high level behavior! Not discussing the desired high level behavior of the kernel in a changelog is a BUG(). - What was the high level design of the old code, was that design fine, should it be changed, and if yes, in what way? Note that we describe the high level design even if we don't change it, partly to give context to the reader, partly to prove that you know what you are doing, to build trust in your patch! Not discussing the old (and new) design of that area of the kernel is a BUG(). and only then do we: - Describe the old implementation, and describe how the new implementation works in all that context. Here is where 99.9% of your existing changelogs fit in. Put differently: your changelogs are missing the first 3 essential components of a changelog. And note, mentioning 'run_size' in a low level description is fine. Mentioning it in a high level description is a BUG(): that is why Boris was trying to change your changelogs into spoken English, to recover some of that missing high level context. Maintainers like Boris should not be forced to do that, you are supposed to offer this with any significant patch, as a passport to prove that your changes to the code are worth looking at. And yes, we do it in that order. Take a look at a recent single line change Linus made in 53da3bc2ba9e48, attached to this mail. Check how the changelog is structured: it discusses high level concepts first. It's a _ONE LINER FIX_ from Linus, yet it's spread over 8 paragraphs and 50 lines, because the change justified that kind of description. And observe the moment when Linus, in his 8 paragraphs, 50 lines description starts talking about low level implementational details, when he mentions lines of code, function names, such as do_numa_page(), 'pte_write()' or 'pte_dirty()'. He doesnt! It's not needed for a one-liner most of the time: but the high level concepts around that code are very much necessary to convey. Now contrast
Re: [PATCH v3 1/7] x86, kaslr: Use init_size instead of run_size
On Sat, Mar 14, 2015 at 08:53:57AM +0100, Ingo Molnar wrote: You are still talking only about the low level code and about low level symptoms, while in contrast to that the primary question with... snip text which will be ignored again, unfortunately Thanks for doing that! I really doubt it'll bring anything this time either but I don't care - certainly I've wasted enough time trying to improve things as far as I can so hell yeah, this bullshit has to stop! In addition to what you've said, I have only one proposal: we revert the whole crap and do it again, this time nice and clean in tip. It'll stay there as long as it has to. No half-arsed commit messages, no misunderstood crap, no lala code f*ckery. Whatever it takes, as long as it takes. Thanks. -- Regards/Gruss, Boris. ECO tip #101: Trim your mails when you reply. -- -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3 1/7] x86, kaslr: Use init_size instead of run_size
On Fri, Mar 13, 2015 at 5:27 AM, Ingo Molnar wrote:
>
> * Yinghai Lu wrote:
>
>> commit e6023367d779 ("x86, kaslr: Prevent .bss from overlaping initrd")
>> introduced one run_size for kaslr.
>> We should use real runtime size (include copy/decompress) aka init_size.
>
> Why, what happens if we don't change this?
While trying to update change log, found we still need to keep this run_size.
otherwise kaslr will find not needed big size of init_size instead of
max(output_len, run_size).
will refresh the patchset.
Thanks
Yinghai
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3 1/7] x86, kaslr: Use init_size instead of run_size
* Yinghai Lu wrote:
> commit e6023367d779 ("x86, kaslr: Prevent .bss from overlaping initrd")
> introduced one run_size for kaslr.
> We should use real runtime size (include copy/decompress) aka init_size.
Why, what happens if we don't change this?
What's the purpose of your change?
> run_size is VO (vmlinux) init size include bss and brk.
> init_size is the size needed for decompress and it is bigger than run_size
> when decompress need more buff.
What happens if we don't have enough 'buff'?
What's the purpose of your change?
> According to arch/x86/boot/header.S:
> | #define ZO_INIT_SIZE(ZO__end - ZO_startup_32 + ZO_z_extract_offset)
> | #define VO_INIT_SIZE(VO__end - VO__text)
> | #if ZO_INIT_SIZE > VO_INIT_SIZE
> | #define INIT_SIZE ZO_INIT_SIZE
> | #else
> | #define INIT_SIZE VO_INIT_SIZE
> | #endif
> | init_size: .long INIT_SIZE # kernel initialization size
>
> Bootloader allocate buffer according to init_size in hdr, and load the
> ZO (arch/x86/boot/compressed/vmlinux) from start of that buffer.
> init_size first come from VO (vmlinux) init size. That VO init size
> is from VO _end to VO _end and include VO bss and brk area.
>
> During running of ZO, ZO move itself to the middle of buffer at
> z_extract_offset to make sure that decompressor would not have output
> overwrite input data before input data get consumed.
> But z_extract_offset calculating is based on size of VO (vmlinux) and size
> of compressed VO only at first.
> So need to make sure [z_extra_offset, init_size) will fit ZO, that means
> init_size need to be adjusted according to ZO size.
> That make init_size is always >= run_size.
>
> During aslr buffer searching, we need to make sure the new buffer is big
> enough for decompress at first. So use init_size instead, and kill not
> needed run_size related code.
Why, what happens if it's not big enough?
What's the purpose of your change?
> Fixes: e6023367d779 ("x86, kaslr: Prevent .bss from overlaping initrd")
What does your patch fix in that commit?
What's the purpose of your change?
After so many words you still haven't explained _the_ most basic thing
a Linux kernel changelog needs to include: why a change is necessary,
i.e. what bad effects did the bug have...
Why are you ignoring the kernel technology of trying to explain things
to others so that they can easily understand so badly?
'Code well explained to others' is just as important a technology to
the Linux kernel as 'correct code', 'clean code' or 'fast code'.
Do you perhaps understand now why I have to ignore most of your
patches after just a sad glance at the changelog's quality?
Thanks,
Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3 1/7] x86, kaslr: Use init_size instead of run_size
On Fri, Mar 13, 2015 at 5:27 AM, Ingo Molnar mi...@kernel.org wrote: * Yinghai Lu ying...@kernel.org wrote: commit e6023367d779 (x86, kaslr: Prevent .bss from overlaping initrd) introduced one run_size for kaslr. We should use real runtime size (include copy/decompress) aka init_size. Why, what happens if we don't change this? While trying to update change log, found we still need to keep this run_size. otherwise kaslr will find not needed big size of init_size instead of max(output_len, run_size). will refresh the patchset. Thanks Yinghai -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3 1/7] x86, kaslr: Use init_size instead of run_size
* Yinghai Lu ying...@kernel.org wrote: commit e6023367d779 (x86, kaslr: Prevent .bss from overlaping initrd) introduced one run_size for kaslr. We should use real runtime size (include copy/decompress) aka init_size. Why, what happens if we don't change this? What's the purpose of your change? run_size is VO (vmlinux) init size include bss and brk. init_size is the size needed for decompress and it is bigger than run_size when decompress need more buff. What happens if we don't have enough 'buff'? What's the purpose of your change? According to arch/x86/boot/header.S: | #define ZO_INIT_SIZE(ZO__end - ZO_startup_32 + ZO_z_extract_offset) | #define VO_INIT_SIZE(VO__end - VO__text) | #if ZO_INIT_SIZE VO_INIT_SIZE | #define INIT_SIZE ZO_INIT_SIZE | #else | #define INIT_SIZE VO_INIT_SIZE | #endif | init_size: .long INIT_SIZE # kernel initialization size Bootloader allocate buffer according to init_size in hdr, and load the ZO (arch/x86/boot/compressed/vmlinux) from start of that buffer. init_size first come from VO (vmlinux) init size. That VO init size is from VO _end to VO _end and include VO bss and brk area. During running of ZO, ZO move itself to the middle of buffer at z_extract_offset to make sure that decompressor would not have output overwrite input data before input data get consumed. But z_extract_offset calculating is based on size of VO (vmlinux) and size of compressed VO only at first. So need to make sure [z_extra_offset, init_size) will fit ZO, that means init_size need to be adjusted according to ZO size. That make init_size is always = run_size. During aslr buffer searching, we need to make sure the new buffer is big enough for decompress at first. So use init_size instead, and kill not needed run_size related code. Why, what happens if it's not big enough? What's the purpose of your change? Fixes: e6023367d779 (x86, kaslr: Prevent .bss from overlaping initrd) What does your patch fix in that commit? What's the purpose of your change? After so many words you still haven't explained _the_ most basic thing a Linux kernel changelog needs to include: why a change is necessary, i.e. what bad effects did the bug have... Why are you ignoring the kernel technology of trying to explain things to others so that they can easily understand so badly? 'Code well explained to others' is just as important a technology to the Linux kernel as 'correct code', 'clean code' or 'fast code'. Do you perhaps understand now why I have to ignore most of your patches after just a sad glance at the changelog's quality? Thanks, Ingo -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3 1/7] x86, kaslr: Use init_size instead of run_size
On Sat, Mar 7, 2015 at 2:07 PM, Yinghai Lu wrote:
> commit e6023367d779 ("x86, kaslr: Prevent .bss from overlaping initrd")
> introduced one run_size for kaslr.
> We should use real runtime size (include copy/decompress) aka init_size.
>
> run_size is VO (vmlinux) init size include bss and brk.
> init_size is the size needed for decompress and it is bigger than run_size
> when decompress need more buff.
>
> According to arch/x86/boot/header.S:
> | #define ZO_INIT_SIZE(ZO__end - ZO_startup_32 + ZO_z_extract_offset)
> | #define VO_INIT_SIZE(VO__end - VO__text)
> | #if ZO_INIT_SIZE > VO_INIT_SIZE
> | #define INIT_SIZE ZO_INIT_SIZE
> | #else
> | #define INIT_SIZE VO_INIT_SIZE
> | #endif
> | init_size: .long INIT_SIZE # kernel initialization size
>
> Bootloader allocate buffer according to init_size in hdr, and load the
> ZO (arch/x86/boot/compressed/vmlinux) from start of that buffer.
> init_size first come from VO (vmlinux) init size. That VO init size
> is from VO _end to VO _end and include VO bss and brk area.
>
> During running of ZO, ZO move itself to the middle of buffer at
> z_extract_offset to make sure that decompressor would not have output
> overwrite input data before input data get consumed.
> But z_extract_offset calculating is based on size of VO (vmlinux) and size
> of compressed VO only at first.
> So need to make sure [z_extra_offset, init_size) will fit ZO, that means
> init_size need to be adjusted according to ZO size.
> That make init_size is always >= run_size.
>
> During aslr buffer searching, we need to make sure the new buffer is big
> enough for decompress at first. So use init_size instead, and kill not
> needed run_size related code.
>
> Fixes: e6023367d779 ("x86, kaslr: Prevent .bss from overlaping initrd")
> Cc: "H. Peter Anvin"
> Cc: Josh Triplett
> Cc: Matt Fleming
> Cc: Kees Cook
> Cc: Andrew Morton
> Cc: Ard Biesheuvel
> Cc: Junjie Mao
> Signed-off-by: Yinghai Lu
Acked-by: Kees Cook
> ---
> arch/x86/boot/compressed/Makefile | 4 +---
> arch/x86/boot/compressed/head_32.S | 5 ++---
> arch/x86/boot/compressed/head_64.S | 5 +
> arch/x86/boot/compressed/misc.c| 15 +++---
> arch/x86/boot/compressed/mkpiggy.c | 9 ++--
> arch/x86/tools/calc_run_size.sh| 42
> --
> 6 files changed, 13 insertions(+), 67 deletions(-)
> delete mode 100644 arch/x86/tools/calc_run_size.sh
>
> diff --git a/arch/x86/boot/compressed/Makefile
> b/arch/x86/boot/compressed/Makefile
> index 0a291cd..70cc92c 100644
> --- a/arch/x86/boot/compressed/Makefile
> +++ b/arch/x86/boot/compressed/Makefile
> @@ -92,10 +92,8 @@ suffix-$(CONFIG_KERNEL_XZ) := xz
> suffix-$(CONFIG_KERNEL_LZO):= lzo
> suffix-$(CONFIG_KERNEL_LZ4):= lz4
>
> -RUN_SIZE = $(shell $(OBJDUMP) -h vmlinux | \
> -$(CONFIG_SHELL) $(srctree)/arch/x86/tools/calc_run_size.sh)
> quiet_cmd_mkpiggy = MKPIGGY $@
> - cmd_mkpiggy = $(obj)/mkpiggy $< $(RUN_SIZE) > $@ || ( rm -f $@ ; false
> )
> + cmd_mkpiggy = $(obj)/mkpiggy $< > $@ || ( rm -f $@ ; false )
>
> targets += piggy.S
> $(obj)/piggy.S: $(obj)/vmlinux.bin.$(suffix-y) $(obj)/mkpiggy FORCE
> diff --git a/arch/x86/boot/compressed/head_32.S
> b/arch/x86/boot/compressed/head_32.S
> index 1d7fbbc..cbed140 100644
> --- a/arch/x86/boot/compressed/head_32.S
> +++ b/arch/x86/boot/compressed/head_32.S
> @@ -207,8 +207,7 @@ relocated:
> * Do the decompression, and jump to the new kernel..
> */
> /* push arguments for decompress_kernel: */
> - pushl $z_run_size /* size of kernel with .bss and .brk */
> - pushl $z_output_len /* decompressed length, end of relocs */
> + pushl $z_output_len /* decompressed length */
> lealz_extract_offset_negative(%ebx), %ebp
> pushl %ebp/* output address */
> pushl $z_input_len/* input_len */
> @@ -218,7 +217,7 @@ relocated:
> pushl %eax/* heap area */
> pushl %esi/* real mode pointer */
> calldecompress_kernel /* returns kernel location in %eax */
> - addl$28, %esp
> + addl$24, %esp
>
> /*
> * Jump to the decompressed kernel.
> diff --git a/arch/x86/boot/compressed/head_64.S
> b/arch/x86/boot/compressed/head_64.S
> index 6b1766c..2884e0c 100644
> --- a/arch/x86/boot/compressed/head_64.S
> +++ b/arch/x86/boot/compressed/head_64.S
> @@ -402,16 +402,13 @@ relocated:
> * Do the decompression, and jump to the new kernel..
> */
> pushq %rsi/* Save the real mode argument */
> - movq$z_run_size, %r9/* size of kernel with .bss and .brk
> */
> - pushq %r9
> movq%rsi, %rdi /* real mode address */
> leaqboot_heap(%rip), %rsi /* malloc area for uncompression */
> leaqinput_data(%rip), %rdx /* input_data */
> movl$z_input_len, %ecx
Re: [PATCH v3 1/7] x86, kaslr: Use init_size instead of run_size
On Mon, Mar 9, 2015 at 1:18 PM, Borislav Petkov wrote: > On Mon, Mar 09, 2015 at 01:06:00PM -0700, Yinghai Lu wrote: >> Yes. Just to emphasize that " We need to make sure [z_extra_offset, >> init_size) will fit ZO" > > So you want to say: > > "We need to make sure the compressed kernel fits in the interval > [z_extra_offset, z_extra_offset + init_size)" > "We need to make sure the compressed kernel fits in the interval [z_extra_offset, init_size)" -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3 1/7] x86, kaslr: Use init_size instead of run_size
On Mon, Mar 09, 2015 at 01:06:00PM -0700, Yinghai Lu wrote: > Yes. Just to emphasize that " We need to make sure [z_extra_offset, > init_size) will fit ZO" So you want to say: "We need to make sure the compressed kernel fits in the interval [z_extra_offset, z_extra_offset + init_size)" ? -- Regards/Gruss, Boris. ECO tip #101: Trim your mails when you reply. -- -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3 1/7] x86, kaslr: Use init_size instead of run_size
On Mon, Mar 9, 2015 at 1:00 PM, Borislav Petkov wrote: > On Mon, Mar 09, 2015 at 12:35:25PM -0700, Yinghai Lu wrote: >> Can you put back: >> " >> So need to make sure [z_extra_offset, init_size) will fit ZO, that means >> init_size need to be adjusted according to ZO size. >> That make init_size is always >= run_size. > > Why? > > We don't adjust init_size. We assign INIT_SIZE to it. Or do you want to > say that we indirectly adjust init_size through INIT_SIZE? Yes. Just to emphasize that " We need to make sure [z_extra_offset, init_size) will fit ZO" -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3 1/7] x86, kaslr: Use init_size instead of run_size
On Mon, Mar 09, 2015 at 12:35:25PM -0700, Yinghai Lu wrote: > Can you put back: > " > So need to make sure [z_extra_offset, init_size) will fit ZO, that means > init_size need to be adjusted according to ZO size. > That make init_size is always >= run_size. Why? We don't adjust init_size. We assign INIT_SIZE to it. Or do you want to say that we indirectly adjust init_size through INIT_SIZE? -- Regards/Gruss, Boris. ECO tip #101: Trim your mails when you reply. -- -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3 1/7] x86, kaslr: Use init_size instead of run_size
On Mon, Mar 9, 2015 at 5:49 AM, Borislav Petkov wrote:
> I ended up committing this. Anything I've missed?
>
> ---
> From: Yinghai Lu
> Date: Sat, 7 Mar 2015 14:07:15 -0800
> Subject: [PATCH] x86/setup: Use init_size instead of run_size
>
> Commit
>
> e6023367d779 ("x86, kaslr: Prevent .bss from overlaping initrd")
>
> introduced run_size for KASLR to represent the size of kernel proper
> (vmlinux).
>
> However, we should use the actual runtime size (which provides for
> copy/decompress), i.e. init_size, as it includes .bss and .brk.
>
> Why, you ask?
>
> Because init_size is the size needed for safe kernel decompression and
> thus can be higher than run_size in case the decompressor needs a larger
> buffer.
>
> From arch/x86/boot/header.S:
> #define ZO_INIT_SIZE(ZO__end - ZO_startup_32 + ZO_z_extract_offset)
> #define VO_INIT_SIZE(VO__end - VO__text)
> #if ZO_INIT_SIZE > VO_INIT_SIZE
> #define INIT_SIZE ZO_INIT_SIZE
> #else
> #define INIT_SIZE VO_INIT_SIZE
> #endif
> init_size: .long INIT_SIZE # kernel initialization size
>
> The boot loader allocates a buffer of size init_size which it
> reads from the setup header and loads the compressed kernel
> (arch/x86/boot/compressed/vmlinux) in it.
>
> init_size initially comes from the kernel proper's (vmlinux) init size.
> It includes the .bss and .brk area.
>
> When the boot loader hands off to the compressed kernel, the last
> moves itself to z_extract_offset within the buffer to make sure that
> the decompressor output does not overwrite input data before it gets
> consumed.
>
> However, z_extract_offset is the size difference
> between the uncompressed and compressed kernel (see
> arch/x86/boot/compressed/mkpiggy.c) and thus represents the additional
> space needed for decompression but it doesn't factor in a bigger
> ZO_INIT_SIZE.
Can you put back:
"
So need to make sure [z_extra_offset, init_size) will fit ZO, that means
init_size need to be adjusted according to ZO size.
That make init_size is always >= run_size.
"
>
> During ASLR buffer searching, we need to make sure the new buffer is big
> enough for decompression. So use init_size instead, and kill run_size
> related code.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3 1/7] x86, kaslr: Use init_size instead of run_size
On Mon, Mar 09, 2015 at 04:58:13PM +0100, Ingo Molnar wrote: > So this would be fine as-is for v4.1, but I think we want to attempt > to fix this in x86/urgent so that we don't have to revert the kaslr > changes in v4.0, so it would be awesome if you could split it into two > parts: the fix, plus the orphaned run_size removal? Ok, lemme do that. -- Regards/Gruss, Boris. ECO tip #101: Trim your mails when you reply. -- -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3 1/7] x86, kaslr: Use init_size instead of run_size
* Borislav Petkov wrote: > On Sat, Mar 07, 2015 at 02:07:15PM -0800, Yinghai Lu wrote: > > ... > > I ended up committing this. Anything I've missed? So this would be fine as-is for v4.1, but I think we want to attempt to fix this in x86/urgent so that we don't have to revert the kaslr changes in v4.0, so it would be awesome if you could split it into two parts: the fix, plus the orphaned run_size removal? That would make it easier to bisect to, should anything break ... Thanks, Ingo -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3 1/7] x86, kaslr: Use init_size instead of run_size
On Sat, Mar 07, 2015 at 02:07:15PM -0800, Yinghai Lu wrote:
...
I ended up committing this. Anything I've missed?
---
From: Yinghai Lu
Date: Sat, 7 Mar 2015 14:07:15 -0800
Subject: [PATCH] x86/setup: Use init_size instead of run_size
Commit
e6023367d779 ("x86, kaslr: Prevent .bss from overlaping initrd")
introduced run_size for KASLR to represent the size of kernel proper
(vmlinux).
However, we should use the actual runtime size (which provides for
copy/decompress), i.e. init_size, as it includes .bss and .brk.
Why, you ask?
Because init_size is the size needed for safe kernel decompression and
thus can be higher than run_size in case the decompressor needs a larger
buffer.
>From arch/x86/boot/header.S:
#define ZO_INIT_SIZE(ZO__end - ZO_startup_32 + ZO_z_extract_offset)
#define VO_INIT_SIZE(VO__end - VO__text)
#if ZO_INIT_SIZE > VO_INIT_SIZE
#define INIT_SIZE ZO_INIT_SIZE
#else
#define INIT_SIZE VO_INIT_SIZE
#endif
init_size: .long INIT_SIZE # kernel initialization size
The boot loader allocates a buffer of size init_size which it
reads from the setup header and loads the compressed kernel
(arch/x86/boot/compressed/vmlinux) in it.
init_size initially comes from the kernel proper's (vmlinux) init size.
It includes the .bss and .brk area.
When the boot loader hands off to the compressed kernel, the last
moves itself to z_extract_offset within the buffer to make sure that
the decompressor output does not overwrite input data before it gets
consumed.
However, z_extract_offset is the size difference
between the uncompressed and compressed kernel (see
arch/x86/boot/compressed/mkpiggy.c) and thus represents the additional
space needed for decompression but it doesn't factor in a bigger
ZO_INIT_SIZE.
During ASLR buffer searching, we need to make sure the new buffer is big
enough for decompression. So use init_size instead, and kill run_size
related code.
Fixes: e6023367d779 ("x86, kaslr: Prevent .bss from overlaping initrd")
Cc: "H. Peter Anvin"
Cc: Josh Triplett
Cc: Matt Fleming
Cc: Kees Cook
Cc: Andrew Morton
Cc: Ard Biesheuvel
Cc: Junjie Mao
Cc: Thomas Gleixner
Cc: Jiri Kosina
Cc: Ingo Molnar
Cc: Baoquan He
Cc: linux-...@vger.kernel.org
Link:
http://lkml.kernel.org/r/1425766041-6551-2-git-send-email-ying...@kernel.org
Signed-off-by: Yinghai Lu
[ Seriously massage commit message. ]
Signed-off-by:
---
arch/x86/boot/compressed/Makefile | 4 +---
arch/x86/boot/compressed/head_32.S | 5 ++---
arch/x86/boot/compressed/head_64.S | 5 +
arch/x86/boot/compressed/misc.c| 15 +++---
arch/x86/boot/compressed/mkpiggy.c | 9 ++--
arch/x86/tools/calc_run_size.sh| 42 --
6 files changed, 13 insertions(+), 67 deletions(-)
delete mode 100644 arch/x86/tools/calc_run_size.sh
diff --git a/arch/x86/boot/compressed/Makefile
b/arch/x86/boot/compressed/Makefile
index 0a291cdfaf77..70cc92c8bcfb 100644
--- a/arch/x86/boot/compressed/Makefile
+++ b/arch/x86/boot/compressed/Makefile
@@ -92,10 +92,8 @@ suffix-$(CONFIG_KERNEL_XZ) := xz
suffix-$(CONFIG_KERNEL_LZO):= lzo
suffix-$(CONFIG_KERNEL_LZ4):= lz4
-RUN_SIZE = $(shell $(OBJDUMP) -h vmlinux | \
-$(CONFIG_SHELL) $(srctree)/arch/x86/tools/calc_run_size.sh)
quiet_cmd_mkpiggy = MKPIGGY $@
- cmd_mkpiggy = $(obj)/mkpiggy $< $(RUN_SIZE) > $@ || ( rm -f $@ ; false )
+ cmd_mkpiggy = $(obj)/mkpiggy $< > $@ || ( rm -f $@ ; false )
targets += piggy.S
$(obj)/piggy.S: $(obj)/vmlinux.bin.$(suffix-y) $(obj)/mkpiggy FORCE
diff --git a/arch/x86/boot/compressed/head_32.S
b/arch/x86/boot/compressed/head_32.S
index 1d7fbbcc196d..cbed1407a5cd 100644
--- a/arch/x86/boot/compressed/head_32.S
+++ b/arch/x86/boot/compressed/head_32.S
@@ -207,8 +207,7 @@ relocated:
* Do the decompression, and jump to the new kernel..
*/
/* push arguments for decompress_kernel: */
- pushl $z_run_size /* size of kernel with .bss and .brk */
- pushl $z_output_len /* decompressed length, end of relocs */
+ pushl $z_output_len /* decompressed length */
lealz_extract_offset_negative(%ebx), %ebp
pushl %ebp/* output address */
pushl $z_input_len/* input_len */
@@ -218,7 +217,7 @@ relocated:
pushl %eax/* heap area */
pushl %esi/* real mode pointer */
calldecompress_kernel /* returns kernel location in %eax */
- addl$28, %esp
+ addl$24, %esp
/*
* Jump to the decompressed kernel.
diff --git a/arch/x86/boot/compressed/head_64.S
b/arch/x86/boot/compressed/head_64.S
index 6b1766c6c082..2884e0c3e8a5 100644
--- a/arch/x86/boot/compressed/head_64.S
+++ b/arch/x86/boot/compressed/head_64.S
@@ -402,16 +402,13 @@ relocated:
* Do the decompression, and jump to the new kernel..
*/
pushq %rsi/* Save the real mode argument */
- movq
Re: [PATCH v3 1/7] x86, kaslr: Use init_size instead of run_size
On Sat, Mar 07, 2015 at 02:07:15PM -0800, Yinghai Lu wrote: ... I ended up committing this. Anything I've missed? --- From: Yinghai Lu ying...@kernel.org Date: Sat, 7 Mar 2015 14:07:15 -0800 Subject: [PATCH] x86/setup: Use init_size instead of run_size Commit e6023367d779 (x86, kaslr: Prevent .bss from overlaping initrd) introduced run_size for KASLR to represent the size of kernel proper (vmlinux). However, we should use the actual runtime size (which provides for copy/decompress), i.e. init_size, as it includes .bss and .brk. Why, you ask? Because init_size is the size needed for safe kernel decompression and thus can be higher than run_size in case the decompressor needs a larger buffer. From arch/x86/boot/header.S: #define ZO_INIT_SIZE(ZO__end - ZO_startup_32 + ZO_z_extract_offset) #define VO_INIT_SIZE(VO__end - VO__text) #if ZO_INIT_SIZE VO_INIT_SIZE #define INIT_SIZE ZO_INIT_SIZE #else #define INIT_SIZE VO_INIT_SIZE #endif init_size: .long INIT_SIZE # kernel initialization size The boot loader allocates a buffer of size init_size which it reads from the setup header and loads the compressed kernel (arch/x86/boot/compressed/vmlinux) in it. init_size initially comes from the kernel proper's (vmlinux) init size. It includes the .bss and .brk area. When the boot loader hands off to the compressed kernel, the last moves itself to z_extract_offset within the buffer to make sure that the decompressor output does not overwrite input data before it gets consumed. However, z_extract_offset is the size difference between the uncompressed and compressed kernel (see arch/x86/boot/compressed/mkpiggy.c) and thus represents the additional space needed for decompression but it doesn't factor in a bigger ZO_INIT_SIZE. During ASLR buffer searching, we need to make sure the new buffer is big enough for decompression. So use init_size instead, and kill run_size related code. Fixes: e6023367d779 (x86, kaslr: Prevent .bss from overlaping initrd) Cc: H. Peter Anvin h...@zytor.com Cc: Josh Triplett j...@joshtriplett.org Cc: Matt Fleming matt.flem...@intel.com Cc: Kees Cook keesc...@chromium.org Cc: Andrew Morton a...@linux-foundation.org Cc: Ard Biesheuvel ard.biesheu...@linaro.org Cc: Junjie Mao eternal@gmail.com Cc: Thomas Gleixner t...@linutronix.de Cc: Jiri Kosina jkos...@suse.cz Cc: Ingo Molnar mi...@redhat.com Cc: Baoquan He b...@redhat.com Cc: linux-...@vger.kernel.org Link: http://lkml.kernel.org/r/1425766041-6551-2-git-send-email-ying...@kernel.org Signed-off-by: Yinghai Lu ying...@kernel.org [ Seriously massage commit message. ] Signed-off-by: --- arch/x86/boot/compressed/Makefile | 4 +--- arch/x86/boot/compressed/head_32.S | 5 ++--- arch/x86/boot/compressed/head_64.S | 5 + arch/x86/boot/compressed/misc.c| 15 +++--- arch/x86/boot/compressed/mkpiggy.c | 9 ++-- arch/x86/tools/calc_run_size.sh| 42 -- 6 files changed, 13 insertions(+), 67 deletions(-) delete mode 100644 arch/x86/tools/calc_run_size.sh diff --git a/arch/x86/boot/compressed/Makefile b/arch/x86/boot/compressed/Makefile index 0a291cdfaf77..70cc92c8bcfb 100644 --- a/arch/x86/boot/compressed/Makefile +++ b/arch/x86/boot/compressed/Makefile @@ -92,10 +92,8 @@ suffix-$(CONFIG_KERNEL_XZ) := xz suffix-$(CONFIG_KERNEL_LZO):= lzo suffix-$(CONFIG_KERNEL_LZ4):= lz4 -RUN_SIZE = $(shell $(OBJDUMP) -h vmlinux | \ -$(CONFIG_SHELL) $(srctree)/arch/x86/tools/calc_run_size.sh) quiet_cmd_mkpiggy = MKPIGGY $@ - cmd_mkpiggy = $(obj)/mkpiggy $ $(RUN_SIZE) $@ || ( rm -f $@ ; false ) + cmd_mkpiggy = $(obj)/mkpiggy $ $@ || ( rm -f $@ ; false ) targets += piggy.S $(obj)/piggy.S: $(obj)/vmlinux.bin.$(suffix-y) $(obj)/mkpiggy FORCE diff --git a/arch/x86/boot/compressed/head_32.S b/arch/x86/boot/compressed/head_32.S index 1d7fbbcc196d..cbed1407a5cd 100644 --- a/arch/x86/boot/compressed/head_32.S +++ b/arch/x86/boot/compressed/head_32.S @@ -207,8 +207,7 @@ relocated: * Do the decompression, and jump to the new kernel.. */ /* push arguments for decompress_kernel: */ - pushl $z_run_size /* size of kernel with .bss and .brk */ - pushl $z_output_len /* decompressed length, end of relocs */ + pushl $z_output_len /* decompressed length */ lealz_extract_offset_negative(%ebx), %ebp pushl %ebp/* output address */ pushl $z_input_len/* input_len */ @@ -218,7 +217,7 @@ relocated: pushl %eax/* heap area */ pushl %esi/* real mode pointer */ calldecompress_kernel /* returns kernel location in %eax */ - addl$28, %esp + addl$24, %esp /* * Jump to the decompressed kernel. diff --git a/arch/x86/boot/compressed/head_64.S b/arch/x86/boot/compressed/head_64.S index 6b1766c6c082..2884e0c3e8a5 100644 ---
Re: [PATCH v3 1/7] x86, kaslr: Use init_size instead of run_size
On Mon, Mar 09, 2015 at 04:58:13PM +0100, Ingo Molnar wrote: So this would be fine as-is for v4.1, but I think we want to attempt to fix this in x86/urgent so that we don't have to revert the kaslr changes in v4.0, so it would be awesome if you could split it into two parts: the fix, plus the orphaned run_size removal? Ok, lemme do that. -- Regards/Gruss, Boris. ECO tip #101: Trim your mails when you reply. -- -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3 1/7] x86, kaslr: Use init_size instead of run_size
* Borislav Petkov b...@suse.de wrote: On Sat, Mar 07, 2015 at 02:07:15PM -0800, Yinghai Lu wrote: ... I ended up committing this. Anything I've missed? So this would be fine as-is for v4.1, but I think we want to attempt to fix this in x86/urgent so that we don't have to revert the kaslr changes in v4.0, so it would be awesome if you could split it into two parts: the fix, plus the orphaned run_size removal? That would make it easier to bisect to, should anything break ... Thanks, Ingo -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3 1/7] x86, kaslr: Use init_size instead of run_size
On Mon, Mar 9, 2015 at 1:18 PM, Borislav Petkov b...@suse.de wrote: On Mon, Mar 09, 2015 at 01:06:00PM -0700, Yinghai Lu wrote: Yes. Just to emphasize that We need to make sure [z_extra_offset, init_size) will fit ZO So you want to say: We need to make sure the compressed kernel fits in the interval [z_extra_offset, z_extra_offset + init_size) We need to make sure the compressed kernel fits in the interval [z_extra_offset, init_size) -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3 1/7] x86, kaslr: Use init_size instead of run_size
On Mon, Mar 9, 2015 at 5:49 AM, Borislav Petkov b...@suse.de wrote: I ended up committing this. Anything I've missed? --- From: Yinghai Lu ying...@kernel.org Date: Sat, 7 Mar 2015 14:07:15 -0800 Subject: [PATCH] x86/setup: Use init_size instead of run_size Commit e6023367d779 (x86, kaslr: Prevent .bss from overlaping initrd) introduced run_size for KASLR to represent the size of kernel proper (vmlinux). However, we should use the actual runtime size (which provides for copy/decompress), i.e. init_size, as it includes .bss and .brk. Why, you ask? Because init_size is the size needed for safe kernel decompression and thus can be higher than run_size in case the decompressor needs a larger buffer. From arch/x86/boot/header.S: #define ZO_INIT_SIZE(ZO__end - ZO_startup_32 + ZO_z_extract_offset) #define VO_INIT_SIZE(VO__end - VO__text) #if ZO_INIT_SIZE VO_INIT_SIZE #define INIT_SIZE ZO_INIT_SIZE #else #define INIT_SIZE VO_INIT_SIZE #endif init_size: .long INIT_SIZE # kernel initialization size The boot loader allocates a buffer of size init_size which it reads from the setup header and loads the compressed kernel (arch/x86/boot/compressed/vmlinux) in it. init_size initially comes from the kernel proper's (vmlinux) init size. It includes the .bss and .brk area. When the boot loader hands off to the compressed kernel, the last moves itself to z_extract_offset within the buffer to make sure that the decompressor output does not overwrite input data before it gets consumed. However, z_extract_offset is the size difference between the uncompressed and compressed kernel (see arch/x86/boot/compressed/mkpiggy.c) and thus represents the additional space needed for decompression but it doesn't factor in a bigger ZO_INIT_SIZE. Can you put back: So need to make sure [z_extra_offset, init_size) will fit ZO, that means init_size need to be adjusted according to ZO size. That make init_size is always = run_size. During ASLR buffer searching, we need to make sure the new buffer is big enough for decompression. So use init_size instead, and kill run_size related code. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3 1/7] x86, kaslr: Use init_size instead of run_size
On Mon, Mar 09, 2015 at 01:06:00PM -0700, Yinghai Lu wrote: Yes. Just to emphasize that We need to make sure [z_extra_offset, init_size) will fit ZO So you want to say: We need to make sure the compressed kernel fits in the interval [z_extra_offset, z_extra_offset + init_size) ? -- Regards/Gruss, Boris. ECO tip #101: Trim your mails when you reply. -- -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3 1/7] x86, kaslr: Use init_size instead of run_size
On Mon, Mar 09, 2015 at 12:35:25PM -0700, Yinghai Lu wrote: Can you put back: So need to make sure [z_extra_offset, init_size) will fit ZO, that means init_size need to be adjusted according to ZO size. That make init_size is always = run_size. Why? We don't adjust init_size. We assign INIT_SIZE to it. Or do you want to say that we indirectly adjust init_size through INIT_SIZE? -- Regards/Gruss, Boris. ECO tip #101: Trim your mails when you reply. -- -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3 1/7] x86, kaslr: Use init_size instead of run_size
On Mon, Mar 9, 2015 at 1:00 PM, Borislav Petkov b...@suse.de wrote: On Mon, Mar 09, 2015 at 12:35:25PM -0700, Yinghai Lu wrote: Can you put back: So need to make sure [z_extra_offset, init_size) will fit ZO, that means init_size need to be adjusted according to ZO size. That make init_size is always = run_size. Why? We don't adjust init_size. We assign INIT_SIZE to it. Or do you want to say that we indirectly adjust init_size through INIT_SIZE? Yes. Just to emphasize that We need to make sure [z_extra_offset, init_size) will fit ZO -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3 1/7] x86, kaslr: Use init_size instead of run_size
On Sat, Mar 7, 2015 at 2:07 PM, Yinghai Lu ying...@kernel.org wrote: commit e6023367d779 (x86, kaslr: Prevent .bss from overlaping initrd) introduced one run_size for kaslr. We should use real runtime size (include copy/decompress) aka init_size. run_size is VO (vmlinux) init size include bss and brk. init_size is the size needed for decompress and it is bigger than run_size when decompress need more buff. According to arch/x86/boot/header.S: | #define ZO_INIT_SIZE(ZO__end - ZO_startup_32 + ZO_z_extract_offset) | #define VO_INIT_SIZE(VO__end - VO__text) | #if ZO_INIT_SIZE VO_INIT_SIZE | #define INIT_SIZE ZO_INIT_SIZE | #else | #define INIT_SIZE VO_INIT_SIZE | #endif | init_size: .long INIT_SIZE # kernel initialization size Bootloader allocate buffer according to init_size in hdr, and load the ZO (arch/x86/boot/compressed/vmlinux) from start of that buffer. init_size first come from VO (vmlinux) init size. That VO init size is from VO _end to VO _end and include VO bss and brk area. During running of ZO, ZO move itself to the middle of buffer at z_extract_offset to make sure that decompressor would not have output overwrite input data before input data get consumed. But z_extract_offset calculating is based on size of VO (vmlinux) and size of compressed VO only at first. So need to make sure [z_extra_offset, init_size) will fit ZO, that means init_size need to be adjusted according to ZO size. That make init_size is always = run_size. During aslr buffer searching, we need to make sure the new buffer is big enough for decompress at first. So use init_size instead, and kill not needed run_size related code. Fixes: e6023367d779 (x86, kaslr: Prevent .bss from overlaping initrd) Cc: H. Peter Anvin h...@zytor.com Cc: Josh Triplett j...@joshtriplett.org Cc: Matt Fleming matt.flem...@intel.com Cc: Kees Cook keesc...@chromium.org Cc: Andrew Morton a...@linux-foundation.org Cc: Ard Biesheuvel ard.biesheu...@linaro.org Cc: Junjie Mao eternal@gmail.com Signed-off-by: Yinghai Lu ying...@kernel.org Acked-by: Kees Cook keesc...@chromium.org --- arch/x86/boot/compressed/Makefile | 4 +--- arch/x86/boot/compressed/head_32.S | 5 ++--- arch/x86/boot/compressed/head_64.S | 5 + arch/x86/boot/compressed/misc.c| 15 +++--- arch/x86/boot/compressed/mkpiggy.c | 9 ++-- arch/x86/tools/calc_run_size.sh| 42 -- 6 files changed, 13 insertions(+), 67 deletions(-) delete mode 100644 arch/x86/tools/calc_run_size.sh diff --git a/arch/x86/boot/compressed/Makefile b/arch/x86/boot/compressed/Makefile index 0a291cd..70cc92c 100644 --- a/arch/x86/boot/compressed/Makefile +++ b/arch/x86/boot/compressed/Makefile @@ -92,10 +92,8 @@ suffix-$(CONFIG_KERNEL_XZ) := xz suffix-$(CONFIG_KERNEL_LZO):= lzo suffix-$(CONFIG_KERNEL_LZ4):= lz4 -RUN_SIZE = $(shell $(OBJDUMP) -h vmlinux | \ -$(CONFIG_SHELL) $(srctree)/arch/x86/tools/calc_run_size.sh) quiet_cmd_mkpiggy = MKPIGGY $@ - cmd_mkpiggy = $(obj)/mkpiggy $ $(RUN_SIZE) $@ || ( rm -f $@ ; false ) + cmd_mkpiggy = $(obj)/mkpiggy $ $@ || ( rm -f $@ ; false ) targets += piggy.S $(obj)/piggy.S: $(obj)/vmlinux.bin.$(suffix-y) $(obj)/mkpiggy FORCE diff --git a/arch/x86/boot/compressed/head_32.S b/arch/x86/boot/compressed/head_32.S index 1d7fbbc..cbed140 100644 --- a/arch/x86/boot/compressed/head_32.S +++ b/arch/x86/boot/compressed/head_32.S @@ -207,8 +207,7 @@ relocated: * Do the decompression, and jump to the new kernel.. */ /* push arguments for decompress_kernel: */ - pushl $z_run_size /* size of kernel with .bss and .brk */ - pushl $z_output_len /* decompressed length, end of relocs */ + pushl $z_output_len /* decompressed length */ lealz_extract_offset_negative(%ebx), %ebp pushl %ebp/* output address */ pushl $z_input_len/* input_len */ @@ -218,7 +217,7 @@ relocated: pushl %eax/* heap area */ pushl %esi/* real mode pointer */ calldecompress_kernel /* returns kernel location in %eax */ - addl$28, %esp + addl$24, %esp /* * Jump to the decompressed kernel. diff --git a/arch/x86/boot/compressed/head_64.S b/arch/x86/boot/compressed/head_64.S index 6b1766c..2884e0c 100644 --- a/arch/x86/boot/compressed/head_64.S +++ b/arch/x86/boot/compressed/head_64.S @@ -402,16 +402,13 @@ relocated: * Do the decompression, and jump to the new kernel.. */ pushq %rsi/* Save the real mode argument */ - movq$z_run_size, %r9/* size of kernel with .bss and .brk */ - pushq %r9 movq%rsi, %rdi /* real mode address */ leaqboot_heap(%rip), %rsi /* malloc area for uncompression */ leaq
