From: Ingo Molnar <mingo@elte.hu>
To: Dan Rosenberg <drosenberg@vsecurity.com>
Cc: Tony Luck <tony.luck@gmail.com>,
linux-kernel@vger.kernel.org, kees.cook@canonical.com,
davej@redhat.com, torvalds@linux-foundation.org,
adobriyan@gmail.com, eranian@google.com, penberg@kernel.org,
davem@davemloft.net, Arjan van de Ven <arjan@infradead.org>,
hpa@zytor.com, Valdis.Kletnieks@vt.edu,
Andrew Morton <akpm@linux-foundation.org>,
pageexec@freemail.hu, Vivek Goyal <vgoyal@redhat.com>
Subject: Re: [RFC][PATCH] Randomize kernel base address on boot
Date: Fri, 27 May 2011 09:15:19 +0200 [thread overview]
Message-ID: <20110527071519.GD9260@elte.hu> (raw)
In-Reply-To: <1306442367.2279.25.camel@dan>
* Dan Rosenberg <drosenberg@vsecurity.com> wrote:
> 5. I'll be switching to per-cpu IDTs, basing my work on the
> following patch:
>
> http://marc.info/?l=linux-kernel&m=112767117501231&w=2
>
> Any review or comments on the above patch would be helpful. I'm
> considering submitting this portion separately, as it may provide
> performance and scalability benefits regardless of randomization.
Yeah.
Note that you do not have to do the MSI thing in Zwane's patch, nor
do i think do you need to touch the boot IDT, but instead go for the
easiest route:
There are two main places that set up the IDT:
trap_init();
init_IRQ()
The IDT is fully set up at this point and i don't think we change it
later on. So all the fancy changes to set_intr_gate() et al in
Zwane's patch seem unnecessary to me.
Most of the complexity Zwane's patch has comes from the fact that he
tries to use per CPU IDTs to create *assymetric* IDTs between CPUs -
but we do not want nor need to do that with your patch, which
simplifies things enormously.
Note that both of the above init functions execute only on the boot
CPU, well before SMP is initialized. So it is an easy environment to
work in from an IDT switcheroo POV and we should be able to switch to
the percpu IDT there without much fuss.
Note that setup_per_cpu_areas() is called well before trap_init(), so
at the end of init_IRQ() you can rely on percpu facilities such as
percpu_alloc() as well.
I'd suggest these rough steps to implement it:
- turn off CONFIG_SMP in the .config
- first add the new init function call to the end of arch/x86/'s
init_IRQ(), put the percpu_alloc() into that function, copy
the old IDT into the new IDT (but do not load it!) and boot test
the patch.
At this point you wont have any change to the IDT yet, but you
have tested all the boot CPU init order assumptions: is
percpu_alloc() really available, did you do the copying right,
etc. You might want to print-dump the new IDT in hexdump format
and check whether it looks like an IDT you'd like the CPU to load.
- then add the one extra line that loads the new IDT into the CPU.
If the kernel does not crash then you will have a randomized UP
kernel that does not leak the randomization secret to user-space
via the SIDT instructon. Test this in user-space, marvel at the
non-kernel-image address you get! :-)
- turn on CONFIG_SMP=y and boot the kernel.
The kernel should not crash: you will have the boot CPU with
the percpu IDT, and all secondary CPUs with the bootup IDT
still referenced. Check via your user-space SIDT test-code
and:
taskset 0 ./test-sidt
taskset 1 ./test-sidt
taskset 2 ./test-sidt
That indeed CPU#0 has a different IDT address from all the other
CPUs. Marvel at the incomplete but still fully working IDT setup!
:-)
- Figure out where a new secondary CPU loads the boot IDT. Figure
out where it sets up its percpu area. Find the spot where both
facilities are available already and add the percpu_alloc()+copy
routine to it. Do the hex printout and boot the kernel - do the
dumped IDTs look sane visually?
- If they looked fine then add the one extra line that loads the
new IDT into the secondary CPU(s). Boot and check the IDTs:
taskset 0 ./test-sidt
taskset 1 ./test-sidt
taskset 2 ./test-sidt
Now you should have different results on all different CPUs!
Marvel at having completed the patch!
- Please check whether the IDT has alignment requirements: we could
actually benefit from coloring the percpu IDTs a bit, as each
hyperthread (and core) has a separate IDT so we can spread out any
cache and RAM accesses a bit better amongst the cache/memory
ports.
- Please check how fast SIDT is, how many cycles does it take? If
it's faster than CPUID then you have also created another nice
scalability feature: a user-space instruction that emits the
current CPU ID! [we could encode the CPU ID in the address - this
will also give us the cache coloring.]
Note that using the percpu area will also avoid the 4K mapping TLB
problem Linus referred to: the percpu area is mapped in a 2MB data
TLB.
What this stage wont allow yet is a read-only IDT. That should be yet
another patch on top of this: the percpu IDT will already allow the
protection of the kernel image randomization secret.
The read-only IDT will bring in the 4K TLB cost but maybe that's
acceptable (because the security advantages of a read-only IDT are
real). It will be a relatively easy patch on top of the percpu IDT
patch: where you load the percpu IDT into the CPU with the LIDT
instruction, you'd first fixmap it into a readonly page:
__set_fixmap(FIX_IDT, __pa(percpu_idt_ptr), PAGE_KERNEL_RO);
And use __fix_to_virt(FIX_IDT) as the load_IDT() address.
If you do it as two patches on top of each other i'll try to figure
out a way to measure the performance impact of the readonly IDT via
perf. It won't be easy as the expected effect is very, very small.
Thanks,
Ingo
next prev parent reply other threads:[~2011-05-27 7:15 UTC|newest]
Thread overview: 95+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-05-24 20:31 [RFC][PATCH] Randomize kernel base address on boot Dan Rosenberg
2011-05-24 21:02 ` Ingo Molnar
2011-05-24 22:55 ` Dan Rosenberg
2011-05-24 21:16 ` Ingo Molnar
2011-05-24 23:00 ` Dan Rosenberg
2011-05-25 11:23 ` Ingo Molnar
2011-05-25 14:20 ` Dan Rosenberg
2011-05-25 14:29 ` Ingo Molnar
2011-05-24 23:06 ` H. Peter Anvin
2011-05-25 14:03 ` Dan Rosenberg
2011-05-25 14:14 ` Ingo Molnar
2011-05-25 15:48 ` H. Peter Anvin
2011-05-25 16:15 ` Dan Rosenberg
2011-05-25 16:24 ` H. Peter Anvin
2011-05-24 21:46 ` Brian Gerst
2011-05-24 23:01 ` Dan Rosenberg
2011-05-24 22:31 ` H. Peter Anvin
2011-05-24 23:04 ` Dan Rosenberg
2011-05-24 23:07 ` H. Peter Anvin
2011-05-24 23:34 ` Dan Rosenberg
2011-05-24 23:36 ` H. Peter Anvin
2011-05-24 23:14 ` H. Peter Anvin
2011-05-24 23:08 ` Dan Rosenberg
2011-05-25 2:05 ` Dan Rosenberg
2011-05-26 20:01 ` Vivek Goyal
2011-05-26 20:06 ` Dan Rosenberg
2011-05-26 20:16 ` Valdis.Kletnieks
2011-05-26 20:31 ` Vivek Goyal
2011-05-27 9:36 ` Ingo Molnar
2011-05-26 20:35 ` Vivek Goyal
2011-05-26 20:40 ` Vivek Goyal
2011-05-26 20:44 ` Dan Rosenberg
2011-05-26 20:55 ` Vivek Goyal
2011-05-27 9:38 ` Ingo Molnar
2011-05-27 13:07 ` Vivek Goyal
2011-05-27 13:38 ` Ingo Molnar
2011-05-27 13:13 ` Vivek Goyal
2011-05-27 13:21 ` Dan Rosenberg
2011-05-27 13:46 ` Ingo Molnar
2011-05-27 13:50 ` Vivek Goyal
2011-05-26 20:39 ` Dan Rosenberg
2011-05-27 7:15 ` Ingo Molnar [this message]
2011-05-31 16:52 ` Matthew Garrett
2011-05-31 18:40 ` H. Peter Anvin
2011-05-31 18:51 ` Matthew Garrett
2011-05-31 19:03 ` Dan Rosenberg
2011-05-31 19:07 ` H. Peter Anvin
2011-05-31 19:50 ` Ingo Molnar
2011-05-31 19:55 ` Ingo Molnar
2011-05-31 20:15 ` H. Peter Anvin
2011-05-31 20:27 ` Ingo Molnar
2011-05-31 20:30 ` H. Peter Anvin
2011-06-01 6:18 ` Ingo Molnar
2011-06-01 15:44 ` H. Peter Anvin
2011-05-31 20:17 ` Dan Rosenberg
2011-05-26 22:18 ` Rafael J. Wysocki
2011-05-26 22:32 ` H. Peter Anvin
2011-05-27 0:26 ` Dan Rosenberg
2011-05-27 16:21 ` Rafael J. Wysocki
2011-05-27 2:45 ` Dave Jones
2011-05-27 9:40 ` Ingo Molnar
2011-05-27 16:11 ` Rafael J. Wysocki
2011-05-27 16:07 ` Rafael J. Wysocki
2011-05-27 15:42 ` Linus Torvalds
2011-05-27 16:11 ` Dan Rosenberg
2011-05-27 17:00 ` Ingo Molnar
2011-05-27 17:06 ` H. Peter Anvin
2011-05-27 17:10 ` Dan Rosenberg
2011-05-27 17:13 ` H. Peter Anvin
2011-05-27 17:16 ` Linus Torvalds
2011-05-27 17:38 ` Ingo Molnar
2011-05-27 17:20 ` Kees Cook
2011-05-27 17:16 ` Ingo Molnar
2011-05-27 17:21 ` Linus Torvalds
2011-05-27 17:46 ` Ingo Molnar
2011-05-27 17:53 ` H. Peter Anvin
2011-05-27 18:05 ` Linus Torvalds
2011-05-27 19:15 ` Vivek Goyal
2011-05-27 21:37 ` H. Peter Anvin
2011-05-27 23:51 ` H. Peter Anvin
2011-05-28 12:18 ` Ingo Molnar
2011-05-29 1:13 ` H. Peter Anvin
2011-05-29 12:47 ` Ingo Molnar
2011-05-29 18:19 ` H. Peter Anvin
2011-05-29 18:44 ` Ingo Molnar
2011-05-29 18:52 ` H. Peter Anvin
2011-05-29 19:56 ` Ingo Molnar
2011-05-27 17:57 ` Linus Torvalds
2011-05-27 18:17 ` Ingo Molnar
2011-05-27 18:43 ` Kees Cook
2011-05-27 18:48 ` david
2011-05-27 21:51 ` Olivier Galibert
2011-05-27 22:11 ` Valdis.Kletnieks
2011-05-28 0:50 ` H. Peter Anvin
2011-05-28 6:32 ` Ingo Molnar
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20110527071519.GD9260@elte.hu \
--to=mingo@elte.hu \
--cc=Valdis.Kletnieks@vt.edu \
--cc=adobriyan@gmail.com \
--cc=akpm@linux-foundation.org \
--cc=arjan@infradead.org \
--cc=davej@redhat.com \
--cc=davem@davemloft.net \
--cc=drosenberg@vsecurity.com \
--cc=eranian@google.com \
--cc=hpa@zytor.com \
--cc=kees.cook@canonical.com \
--cc=linux-kernel@vger.kernel.org \
--cc=pageexec@freemail.hu \
--cc=penberg@kernel.org \
--cc=tony.luck@gmail.com \
--cc=torvalds@linux-foundation.org \
--cc=vgoyal@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox