Re: [PATCH v6 09/10] powerpc/64s: Implement KUAP for Radix MMU

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

Le 17/04/2020 à 12:39, Nicholas Piggin a écrit :
> Excerpts from Christophe Leroy's message of April 17, 2020 8:10 pm:
>>
>>
>> Le 18/04/2019 à 08:51, Michael Ellerman a écrit :
>>> Kernel Userspace Access Prevention utilises a feature of the Radix MMU
>>> which disallows read and write access to userspace addresses. By
>>> utilising this, the kernel is prevented from accessing user data from
>>> outside of trusted paths that perform proper safety checks, such as
>>> copy_{to/from}_user() and friends.
>>>
>>> Userspace access is disabled from early boot and is only enabled when
>>> performing an operation like copy_{to/from}_user(). The register that
>>> controls this (AMR) does not prevent userspace from accessing itself,
>>> so there is no need to save and restore when entering and exiting
>>> userspace.
>>>
>>> When entering the kernel from the kernel we save AMR and if it is not
>>> blocking user access (because eg. we faulted doing a user access) we
>>> reblock user access for the duration of the exception (ie. the page
>>> fault) and then restore the AMR when returning back to the kernel.
>>>
>>> This feature can be tested by using the lkdtm driver (CONFIG_LKDTM=y)
>>> and performing the following:
>>>
>>>     # (echo ACCESS_USERSPACE) > [debugfs]/provoke-crash/DIRECT
>>>
>>> If enabled, this should send SIGSEGV to the thread.
>>>
>>> We also add paranoid checking of AMR in switch and syscall return
>>> under CONFIG_PPC_KUAP_DEBUG.
>>>
>>> Co-authored-by: Michael Ellerman <mpe@ellerman.id.au>
>>> Signed-off-by: Russell Currey <ruscur@russell.cc>
>>> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
>>
>> [...]
>>
>>> diff --git a/arch/powerpc/kernel/entry_64.S b/arch/powerpc/kernel/entry_64.S
>>> index 15c67d2c0534..7cc25389c6bd 100644
>>> --- a/arch/powerpc/kernel/entry_64.S
>>> +++ b/arch/powerpc/kernel/entry_64.S
>>
>> [...]
>>
>>> @@ -594,6 +606,8 @@ _GLOBAL(_switch)
>>>    	std	r23,_CCR(r1)
>>>    	std	r1,KSP(r3)	/* Set old stack pointer */
>>>    
>>> +	kuap_check_amr r9, r10
>>> +
>>>    	FLUSH_COUNT_CACHE
>>>    
>>>    	/*
>>
>> I'm having a problem with this check. As you know I implemented the
>> exact same check in _switch() in entry_32.S. After adding some printk
>> inside an user_access_begin()/user_access_end() section, I started to
>> get valid user accesses blocked by KUAP. Then I activated
>> CONFIG_PPC_KUAP_DEBUG which led me to WARNINGs on this check.
>>
>> This is due to schedule() being called by printk() inside the section
>> where user access is unlocked. How is this supposed to work ? When
> 
> Unlocked user access sections are supposed to be very constrained,
> I think x86's objtool has a checker to verify nothing much gets
> called. Printk shouldn't be.
> 
> I was hitting the same assertion and it was because the uaccess
> macros were pulling lots of things into the user access region.
> 
> https://patchwork.ozlabs.org/project/linuxppc-dev/patch/20200407041245.600651-1-npiggin@gmail.com/
> 
> If that doesn't solve your problem... then now is printk being
> called with user access enabled?

Yes indeed, the printk was called with user access enabled, as I wanted 
to print all calls to unsafe_copy_to_user()

Thanks
Christophe