[v4] powerpc: Avoid code patching freed init sections

[Michael Neuling]

On Mon, 2018-10-01 at 13:25 +0200, Christophe LEROY wrote:
> 
> 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

Arrh, sorry.

Can you write this up as a real patch with a signed off by so mpe can take it?

Mikey

> 
> 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
>