On Sun, 2017-05-28 at 17:59 +0200, christophe leroy wrote:

Le 25/05/2017 à 05:36, Balbir Singh a écrit :

quoted

Today our patching happens via direct copy and
patch_instruction. The patching code is well
contained in the sense that copying bits are limited.

While considering implementation of CONFIG_STRICT_RWX,
the first requirement is to a create another mapping
that will allow for patching. We create the window using
text_poke_area, allocated via get_vm_area(), which might
be an overkill. We can do per-cpu stuff as well. The
downside of these patches that patch_instruction is
now synchornized using a lock. Other arches do similar
things, but use fixmaps. The reason for not using
fixmaps is to make use of any randomization in the
future. The code also relies on set_pte_at and pte_clear
to do the appropriate tlb flushing.

Isn't it overkill to remap the text in another area ?

Among the 6 arches implementing CONFIG_STRICT_KERNEL_RWX (arm, arm64, 
parisc, s390, x86/32, x86/64):
- arm, x86/32 and x86/64 set text RW during the modification

x86 uses set_fixmap() in text_poke(), am I missing something?

- s390 seems to uses a special instruction which bypassed write protection
- parisc doesn't seem to implement any function which modifies kernel text.

Therefore it seems only arm64 does it via another mapping.
Wouldn't it be lighter to just unprotect memory during the modification 
as done on arm and x86 ?

I am not sure if the trade-off is quite that simple, for security I thought

1. It would be better to randomize text_poke_area(), which is why I dynamically
allocated it. If we start randomizing get_vm_area(), we get the benefit
2. text_poke_aread() is RW and the normal text is RX, for any attack
to succeed, it would need to find text_poke_area() at the time of patching,
patch the kernel in that small window and use the normal mapping for
execution

Generally patch_instruction() is not fast path except for ftrace, tracing.
In my tests I did not find the slow down noticable

Or another alternative could be to disable DMMU and do the write at 
physical address ?

This would be worse off, I think, but we were discussing doing something
like that xmon. But for other cases, I think  it opens up a bigger window.

Christophe

Balbir Singh