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: sta...@vger.kernel.org # 4.13+
Signed-off-by: Michael Neuling <mi...@neuling.org>
Reviewed-by: Nicholas Piggin <npig...@gmail.com>
Reviewed-by: Christophe Leroy <christophe.le...@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

Reply via email to