linux-kernel - Re: [PATCH v3 5/6] x86/alternative: Use a single access in text

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <73A1EC50-AF51-4035-A3B3-D989AC89B929@vmware.com>
Date:   Fri, 11 Jan 2019 17:54:43 +0000
From:   Nadav Amit <namit@...are.com>
To:     Jason Baron <jbaron@...mai.com>
CC:     Josh Poimboeuf <jpoimboe@...hat.com>,
        Alexandre Chartre <alexandre.chartre@...cle.com>,
        Sean Christopherson <sean.j.christopherson@...el.com>,
        Steven Rostedt <rostedt@...dmis.org>, X86 ML <x86@...nel.org>,
        LKML <linux-kernel@...r.kernel.org>,
        Ard Biesheuvel <ard.biesheuvel@...aro.org>,
        Andy Lutomirski <luto@...nel.org>,
        Peter Zijlstra <peterz@...radead.org>,
        Ingo Molnar <mingo@...nel.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        Masami Hiramatsu <mhiramat@...nel.org>,
        Jiri Kosina <jkosina@...e.cz>,
        David Laight <David.Laight@...LAB.COM>,
        Borislav Petkov <bp@...en8.de>,
        Julia Cartwright <julia@...com>, Jessica Yu <jeyu@...nel.org>,
        "H. Peter Anvin" <hpa@...or.com>,
        Rasmus Villemoes <linux@...musvillemoes.dk>,
        Edward Cree <ecree@...arflare.com>,
        Daniel Bristot de Oliveira <bristot@...hat.com>
Subject: Re: [PATCH v3 5/6] x86/alternative: Use a single access in
 text_poke() where possible

> On Jan 11, 2019, at 9:41 AM, Jason Baron <jbaron@...mai.com> wrote:
> 
> On 1/11/19 11:57 AM, Josh Poimboeuf wrote:
>> On Fri, Jan 11, 2019 at 05:46:36PM +0100, Alexandre Chartre wrote:
>>> On 01/11/2019 04:28 PM, Josh Poimboeuf wrote:
>>>> On Fri, Jan 11, 2019 at 01:10:52PM +0100, Alexandre Chartre wrote:
>>>>> To avoid any issue with live patching the call instruction, what about
>>>>> toggling between two call instructions: one would be the currently active
>>>>> call, while the other would currently be inactive but to be used after a
>>>>> change is made. You can safely patch the inactive call and then toggle
>>>>> the call flow (using a jump label) between the active and inactive calls.
>>>>> 
>>>>> So instead of having a single call instruction:
>>>>> 
>>>>> 	call function
>>>>> 
>>>>> You would have:
>>>>> 
>>>>> 	STATIC_JUMP_IF_TRUE label, key
>>>>> 	call function1
>>>>> 	jmp done
>>>>> label:
>>>>> 	call function2
>>>>> done:
>>>>> 
>>>>> If the key is set so that function1 is currently called then you can
>>>>> safely update the call instruction for function2. Once this is done,
>>>>> just flip the key to make the function2 call active. On a next update,
>>>>> you would, of course, have to switch and update the call for function1.
>>>> 
>>>> What about the following race?
>>>> 
>>>> CPU1						CPU2
>>>> static key is false, doesn't jump
>>>> task gets preempted before calling function1
>>>> 						change static key to true
>>>> 						start patching "call function1"
>>>> task resumes, sees inconsistent call instruction
>>> 
>>> If the function1 call is active then it won't be changed, you will change
>>> function2. However, I presume you can still have a race but if the function
>>> is changed twice before calling function1:
>>> 
>>> CPU1						CPU2
>>> static key is false, doesn't jump
>>> task gets preempted before calling function1
>>>                                                -- first function change --
>>>                                                patch "call function2"
>>>                                                change static key to true
>>>                                                -- second function change --
>>>                                                start patching "call function1"
>>> task resumes, sees inconsistent call instruction
>>> 
>>> So right, that's a problem.
>> 
>> Right, that's what I meant to say :-)
> 
> could you use something like synchronize_rcu_tasks() between successive
> updates to guarantee nobody's stuck in the middle of the call
> instruction update? Yes its really slow but the update path is slow anyways.

You would need to disable preemption or IRQs before the call, which is not
something you want to do (when would you enable it?)

Having said that, I suggested something somewhat similar things here:
https://lore.kernel.org/lkml/F6735FF5-4D62-4C4E-A145-751E6469CE9E@vmware.com/
https://lore.kernel.org/lkml/CCF7D3C7-9D12-489B-B778-C2156D2DBF47@vmware.com/