[v4] powerpc: Avoid code patching freed init sections

[Christophe LEROY]

Le 21/09/2018 à 13:59, Michael Ellerman a écrit :
> On Fri, 2018-09-14 at 01:14:11 UTC, Michael Neuling wrote:
>> This stops us from doing code patching in init sections after they've
>> been freed.
>>
>> In this chain:
>>    kvm_guest_init() ->
>>      kvm_use_magic_page() ->
>>        fault_in_pages_readable() ->
>> 	 __get_user() ->
>> 	   __get_user_nocheck() ->
>> 	     barrier_nospec();
>>
>> We have a code patching location at barrier_nospec() and
>> kvm_guest_init() is an init function. This whole chain gets inlined,
>> so when we free the init section (hence kvm_guest_init()), this code
>> goes away and hence should no longer be patched.
>>
>> We seen this as userspace memory corruption when using a memory
>> checker while doing partition migration testing on powervm (this
>> starts the code patching post migration via
>> /sys/kernel/mobility/migration). In theory, it could also happen when
>> using /sys/kernel/debug/powerpc/barrier_nospec.
>>
>> cc: stable at vger.kernel.org # 4.13+
>> Signed-off-by: Michael Neuling <mikey at neuling.org>
>> Reviewed-by: Nicholas Piggin <npiggin at gmail.com>
>> Reviewed-by: Christophe Leroy <christophe.leroy at c-s.fr>
> 
> Applied to powerpc fixes, thanks.
> 
> https://git.kernel.org/powerpc/c/51c3c62b58b357e8d35e4cc32f7b4e
> 

This patch breaks booting on my MPC83xx board (book3s/32) very early 
(before console is active), provoking restart.
u-boot reports a checkstop reset at restart.

Reverting this commit fixes the issue.

The following patch fixes the issue as well, but I think it is not the 
best solution. I still think the test should be in patch_instruction() 
instead of being in __patch_instruction(), see my comment on v2

Christophe

diff --git a/arch/powerpc/lib/code-patching.c 
b/arch/powerpc/lib/code-patching.c
index 6ae2777..6192fda 100644
--- a/arch/powerpc/lib/code-patching.c
+++ b/arch/powerpc/lib/code-patching.c
@@ -29,7 +29,7 @@ static int __patch_instruction(unsigned int 
*exec_addr, unsigned int instr,
         int err;

         /* Make sure we aren't patching a freed init section */
-       if (init_mem_is_free && init_section_contains(exec_addr, 4)) {
+       if (*PTRRELOC(&init_mem_is_free) && 
init_section_contains(exec_addr, 4)) {
                 pr_debug("Skipping init section patching addr: 
0x%px\n", exec_addr);
                 return 0;
         }


Christophe