linux-kernel - Re: [PATCH v2 07/12] x86: add new features for paravirt patching

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: Windows password security audit tool. GUI, reports in PDF.

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

Date:   Wed, 9 Dec 2020 13:22:24 +0100
From:   Jürgen Groß <jgross@...e.com>
To:     Borislav Petkov <bp@...en8.de>
Cc:     xen-devel@...ts.xenproject.org, x86@...nel.org,
        linux-kernel@...r.kernel.org, peterz@...radead.org,
        luto@...nel.org, Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>,
        "H. Peter Anvin" <hpa@...or.com>
Subject: Re: [PATCH v2 07/12] x86: add new features for paravirt patching

On 09.12.20 13:03, Borislav Petkov wrote:
> On Wed, Dec 09, 2020 at 08:30:53AM +0100, Jürgen Groß wrote:
>> Hey, I already suggested to use ~FEATURE for that purpose (see
>> https://lore.kernel.org/lkml/f105a63d-6b51-3afb-83e0-e899ea40813e@suse.com/
> 
> Great minds think alike!
> 
> :-P
> 
>> I'd rather make the syntax:
>>
>> ALTERNATIVE_TERNARY <initial-code> <feature> <code-for-feature-set>
>>                                               <code-for-feature-unset>
>>
>> as this ...
> 
> Sure, that is ok too.
> 
>> ... would match perfectly to this interpretation.
> 
> Yap.
> 
>> Hmm, using flags is an alternative (pun intended :-) ).
> 
> LOL!
> 
> Btw, pls do check how much the vmlinux size of an allyesconfig grows
> with this as we will be adding a byte per patch site. Not that it would
> matter too much - the flags are a long way a comin'. :-)
> 
>> No, this is needed for non-Xen cases, too (especially pv spinlocks).
> 
> I see.
> 
>>> Can you give an example here pls why the paravirt patching needs to run
>>> first?
>>
>> Okay.
> 
> I meant an example for me to have a look at. :)
> 
> If possible pls.

Ah, okay.

Lets take the spin_unlock() case. With patch 11 of the series this is

PVOP_ALT_VCALLEE1(lock.queued_spin_unlock, lock,
                   "movb $0, (%%" _ASM_ARG1 ");",
                   X86_FEATURE_NO_PVUNLOCK);

which boils down to ALTERNATIVE "call *lock.queued_spin_unlock"
                                 "movb $0,(%rdi)" X86_FEATURE_NO_PVUNLOCK

The initial (paravirt) code is an indirect call in order to allow
spin_unlock() before paravirt/alternative patching takes place.

Paravirt patching will then replace the indirect call with a direct call
to the correct unlock function. Then alternative patching might replace
the direct call to the bare metal unlock with a plain "movb $0,(%rdi)"
in case pvlocks are not enabled.

In case alternative patching would occur first, the indirect call might
be replaced with the "movb ...", and then paravirt patching would
clobber that with the direct call, resulting in the bare metal
optimization being removed again.


Juergen

Download attachment "OpenPGP_0xB0DE9DD628BF132F.asc" of type "application/pgp-keys" (3092 bytes)

Download attachment "OpenPGP_signature" of type "application/pgp-signature" (496 bytes)