Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1380167 > unrolled thread

Re: [PATCH v5 07/21] x86, boot: Fix run_size calculation

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH v5 07/21] x86, boot: Fix run_size calculation Kees Cook <keescook@chromium.org> - 2016-04-15 21:30 +0200
    Re: [PATCH v5 07/21] x86, boot: Fix run_size calculation Ingo Molnar <mingo@kernel.org> - 2016-04-16 11:10 +0200

#1380167 — Re: [PATCH v5 07/21] x86, boot: Fix run_size calculation

On Fri, Apr 15, 2016 at 1:31 AM, Ingo Molnar <mingo@kernel.org> wrote:
>
> * Kees Cook <keescook@chromium.org> wrote:
>
>> From: Yinghai Lu <yinghai@kernel.org>
>>
>> Currently, the kernel run_size (size of code plus brk and bss) is
>> calculated via the shell script arch/x86/tools/calc_run_size.sh.
>> It gets the file offset and mem size of for the .bss and .brk sections in
>> vmlinux, and adds them as follows:
>
> 'of for'?
>
>> So, from this we can see that the existing run_size calculation is
>> 0x400000 too high.
>
> Btw., why is it too high? Were some sections discarded?

I'm not sure why it's that specific value, but the old method used the
section's ELF _file_ position, not the virtual address, and that
seemed to cause the problem.

>
>> [...] And, as it turns out, the correct run_size is
>> actually equal to VO_end - VO_text, which is certainly easier to calculate.
>> _end: 0xffffffff8205f000
>> _text:0xffffffff81000000
>>
>> 0xffffffff8205f000 - 0xffffffff81000000 = 0x105f000
>>
>> As a result, run_size is a simple constant, so we don't need to pass it
>> around; we already have voffset.h such things.
>
> (Typo.)
>
>> [...] We can share voffset.h
>> between misc.c and header.S instead of getting run_size in other ways.
>> This patch moves voffset.h creation code to boot/compressed/Makefile,
>> and switches misc.c to use the VO_end - VO_text calculation.
>
> Btw., 'run_size' is a pretty meaningless name, it doesn't really tell us the most
> important thing: that this is the complete size of the uncompressed kernel, the
> total physically continuous size we need to allocate to it so it can execute?
>
> So can we rename it to something more expressive, such as kernel_total_size or so?

You got it. Thanks again for digging through all this!

-Kees

-- 
Kees Cook
Chrome OS & Brillo Security

[toc] | [next] | [standalone]

#1380513

* Kees Cook <keescook@chromium.org> wrote:

> > So can we rename it to something more expressive, such as kernel_total_size or 
> > so?
> 
> You got it. Thanks again for digging through all this!

You are welcome! A couple of logistical suggestions:

Could you please split up the series a bit and limit the next series to say no 
more than around 5 patches? (Can be a little bit more when justified to finish up 
a particular line of thought) That way I can apply them in reviewable groups, 
without having to reject the whole series because some patch deep into the series 
has some problem.

I'd suggest starting with absolutely critical fixes (if any!) as-is, to make 
backporting easier. By the looks of it I don't think there's any such patch in 
this series, but just in case there are any, they can be at the front.

Then come the various cleanup patches and non-critical fixes - everything that is 
not supposed to change the behavior of the kernel. I'd suggest doing them in 
roughly this order:

 - file renames first - so that any later revert in a smaller patch does not have
   to go through a rename barrier.

 - then .o-invariant trivial cleanups, the fixing, harmonization (and creation ;-)
   of comments.

 - then come more involved cleanups like moving logic from build time to boot
   time, stricter bounds checks, non-essential fixes, etc.

It might be useful if you declared at this stage that you are mostly done with the 
preparatory work and that the code base is ready for heavier changes, so that 
people (and me) can review the whole source for anything missing. Often a car 
needs a good power wash before we can tell what body work is needed.

... and once we are happy and proud about the code base, then come the more 
exciting things: more fundamental changes, and new features - on top of a squeaky 
clean code base.

This all can happen pretty quickly, as long as the ordering is proper.

Thanks,

	Ingo

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web