Re: [PATCH] intel_txt: add s3 userspace memory integrity verification

[Pavel Machek <pavel@ucw.cz>]

On Fri 2009-12-04 14:24:24, H. Peter Anvin wrote:
> On 12/04/2009 02:15 PM, Pavel Machek wrote:
> >>>
> >>> Are  you sure x86-64 kernel & modules is always below 4GB? I don't
> >>> think so.
> 
> The x86-64 kernel is run where it is loaded by the boot loader.  For
> most boot loaders, that will mean < 4 GB.  This is not the case for
> modules, and they cannot and should not rely on modules inside
> restricted zone.
> 
> This effectively becomes a constraint on whatever boot loader is used to
> load the kernel for it to be compatible with tboot.

Having "security" technology that silently fails with funny bootloader
is pretty bad, I'd say.

Instead of doing this properly (in tboot), Joseph hopes to save some
work by basically splitting kernel into two parts, "trusted" and
"untrusted". But doing that properly would be too much work, so he
just handwaves and hopes for the best.

Unfortunately that

a) does not work (panic, printks)

b) places funny constraints all over the code (documenting them would
be too much work, so we get more handwaving)

c) reduces future flexibility (trusted code can not be over 4GB, it is
silent security hole when it is) 

I'd prefer to see this done properly; tboot should simply verify all
the memory for us. It is separate piece of code, trusted, and can
probably fit itself under 4GB.

If that seems like too much work, then please go all over the code,
and mark all the code that is "trusted" (has to be under 4GB) and
properly document it, so that future modifications will not break that
assumption. Putting all that code into one section should be enough.

(Going into infinite loop is probably enough when memory corruption is
detected; there's no chance you can put printk/panic/neccessary
drivers all into the "trusted" section.)

Aha, and look. Your tboot_gen_mem_integrity is ran on resume, so it is
"trusted". Unfortunately, it calls crypto_alloc_hash, so you need to
audit that, and probably kmalloc it boils down into. I bet kmalloc
touches memory >4GB. And I bet crypto modules can be
... well... modules. That means over 4GB.

This is broken by design, right?
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html