From: Christophe LEROY <christophe.leroy@c-s.fr>
To: Michael Ellerman <patch-notifications@ellerman.id.au>,
Michael Neuling <mikey@neuling.org>
Cc: "Michal Suchánek" <msuchanek@suse.de>,
linuxppc-dev@lists.ozlabs.org,
"Nicholas Piggin" <npiggin@gmail.com>,
"Haren Myneni" <haren@linux.vnet.ibm.com>
Subject: Re: [v4] powerpc: Avoid code patching freed init sections
Date: Mon, 1 Oct 2018 13:25:46 +0200 [thread overview]
Message-ID: <0702294f-6a17-18aa-95f6-d97da4596dbd@c-s.fr> (raw)
In-Reply-To: <42GsbJ0Knkz9sCh@ozlabs.org>
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@vger.kernel.org # 4.13+
>> Signed-off-by: Michael Neuling <mikey@neuling.org>
>> Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
>> Reviewed-by: Christophe Leroy <christophe.leroy@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
next prev parent reply other threads:[~2018-10-01 11:27 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-09-14 1:14 [PATCH v4] powerpc: Avoid code patching freed init sections Michael Neuling
2018-09-14 4:22 ` Nicholas Piggin
2018-09-18 8:52 ` Christophe LEROY
2018-09-18 11:35 ` Michal Suchánek
2018-09-14 5:32 ` Christophe LEROY
2018-09-21 11:59 ` [v4] " Michael Ellerman
2018-10-01 11:25 ` Christophe LEROY [this message]
2018-10-01 22:57 ` Michael Neuling
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=0702294f-6a17-18aa-95f6-d97da4596dbd@c-s.fr \
--to=christophe.leroy@c-s.fr \
--cc=haren@linux.vnet.ibm.com \
--cc=linuxppc-dev@lists.ozlabs.org \
--cc=mikey@neuling.org \
--cc=msuchanek@suse.de \
--cc=npiggin@gmail.com \
--cc=patch-notifications@ellerman.id.au \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.