Re: [RFC PATCH 0/3] Use per-CPU temporary mappings for patching

[Christophe Leroy <christophe.leroy@c-s.fr>]

Le 23/03/2020 à 15:04, Christophe Leroy a écrit :
> 
> 
> On 03/23/2020 11:30 AM, Christophe Leroy wrote:
>>
>>
>> On 03/23/2020 04:52 AM, Christopher M. Riedl wrote:
>>> When compiled with CONFIG_STRICT_KERNEL_RWX, the kernel must create
>>> temporary mappings when patching itself. These mappings temporarily
>>> override the strict RWX text protections to permit a write. Currently,
>>> powerpc allocates a per-CPU VM area for patching. Patching occurs as
>>> follows:
>>>
>>>     1. Map page of text to be patched to per-CPU VM area w/
>>>        PAGE_KERNEL protection
>>>     2. Patch text
>>>     3. Remove the temporary mapping
>>>
>>> While the VM area is per-CPU, the mapping is actually inserted into the
>>> kernel page tables. Presumably, this could allow another CPU to access
>>> the normally write-protected text - either malicously or accidentally -
>>> via this same mapping if the address of the VM area is known. Ideally,
>>> the mapping should be kept local to the CPU doing the patching (or any
>>> other sensitive operations requiring temporarily overriding memory
>>> protections) [0].
>>>
>>> x86 introduced "temporary mm" structs which allow the creation of
>>> mappings local to a particular CPU [1]. This series intends to bring the
>>> notion of a temporary mm to powerpc and harden powerpc by using such a
>>> mapping for patching a kernel with strict RWX permissions.
>>>
>>> The first patch introduces the temporary mm struct and API for powerpc
>>> along with a new function to retrieve a current hw breakpoint.
>>>
>>> The second patch uses the `poking_init` init hook added by the x86
>>> patches to initialize a temporary mm and patching address. The patching
>>> address is randomized between 0 and DEFAULT_MAP_WINDOW-PAGE_SIZE. The
>>> upper limit is necessary due to how the hash MMU operates - by default
>>> the space above DEFAULT_MAP_WINDOW is not available. For now, both hash
>>> and radix randomize inside this range. The number of possible random
>>> addresses is dependent on PAGE_SIZE and limited by DEFAULT_MAP_WINDOW.
>>>
>>> Bits of entropy with 64K page size on BOOK3S_64:
>>>
>>>     bits-o-entropy = log2(DEFAULT_MAP_WINDOW_USER64 / PAGE_SIZE)
>>>
>>>     PAGE_SIZE=64K, DEFAULT_MAP_WINDOW_USER64=128TB
>>>     bits-o-entropy = log2(128TB / 64K)
>>>     bits-o-entropy = 31
>>>
>>> Currently, randomization occurs only once during initialization at boot.
>>>
>>> The third patch replaces the VM area with the temporary mm in the
>>> patching code. The page for patching has to be mapped PAGE_SHARED with
>>> the hash MMU since hash prevents the kernel from accessing userspace
>>> pages with PAGE_PRIVILEGED bit set. There is on-going work on my side to
>>> explore if this is actually necessary in the hash codepath.
>>>
>>> Testing so far is limited to booting on QEMU (power8 and power9 targets)
>>> and a POWER8 VM along with setting some simple xmon breakpoints (which
>>> makes use of code-patching). A POC lkdtm test is in-progress to actually
>>> exploit the existing vulnerability (ie. the mapping during patching is
>>> exposed in kernel page tables and accessible by other CPUS) - this will
>>> accompany a future v1 of this series.
>>
>> Got following failures on an 8xx. Note that "fault blocked by AP 
>> register !" means an unauthorised access from Kernel to Userspace.
>>
> 
> Still a problem even without CONFIG_PPC_KUAP:
> 

I've been able to dig into the problem.

With CONFIG_PPC_KUAP, it can definitely not work. See why in commit 
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?id=ef296729b735e083d8919e76ac213b8ff237eb78

Without CONFIG_PPC_KUAP, on the 8xx, __put_user_asm() in 
__patch_instruction() returns -EFAULT. That's because _PAGE_DIRTY is not 
set on the page. Normally it should be a minor fault and the fault 
handler should set the _PAGE_DIRTY flag. It must be something in the way 
the page is allocated and mapped which prevents that. If I forge 
_PAGE_DIRTY in addition to PAGE_SHARED, it works. But I don't think it 
is valid approach to solve the issue.

Christophe