linux-kernel - Re: [PATCH v2 10/11] x86/alternatives: Simplify ALTERNATIVE

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]

Message-ID: <20230815224419.GF971582@hirez.programming.kicks-ass.net>
Date:   Wed, 16 Aug 2023 00:44:19 +0200
From:   Peter Zijlstra <peterz@...radead.org>
To:     Nikolay Borisov <nik.borisov@...e.com>
Cc:     x86@...nel.org, linux-kernel@...r.kernel.org, David.Kaplan@....com,
        Andrew.Cooper3@...rix.com, jpoimboe@...nel.org,
        gregkh@...uxfoundation.org
Subject: Re: [PATCH v2 10/11] x86/alternatives: Simplify ALTERNATIVE_n()

On Tue, Aug 15, 2023 at 11:49:16PM +0300, Nikolay Borisov wrote:
> 
> 
> On 14.08.23 г. 14:44 ч., Peter Zijlstra wrote:
> > Instead of making increasingly complicated ALTERNATIVE_n()
> > implementations, use a nested alternative expression.
> > 
> > The only difference between:
> > 
> >    ALTERNATIVE_2(oldinst, newinst1, flag1, newinst2, flag2)
> > 
> > and
> > 
> >    ALTERNATIVE(ALTERNATIVE(oldinst, newinst1, flag1),
> >                newinst2, flag2)
> > 
> > is that the outer alternative can add additional padding when the
> > inner alternative is the shorter one, which then results in
> > alt_instr::instrlen being inconsistent.
> > 
> > However, this is easily remedied since the alt_instr entries will be
> > consecutive and it is trivial to compute the max(alt_instr::instrlen)
> > at runtime while patching.
> > 
> > Specifically, after this patch the ALTERNATIVE_2 macro, after CPP
> > expansion (and manual layout), looks like this:
> > 
> >    .macro ALTERNATIVE_2 oldinstr, newinstr1, ft_flags1, newinstr2, ft_flags2
> >     140:
> > 
> >       140: \oldinstr ;
> >       141: .skip -(((144f-143f)-(141b-140b)) > 0) * ((144f-143f)-(141b-140b)),0x90 ;
> >       142: .pushsection .altinstructions,"a" ;
> > 	  altinstr_entry 140b,143f,\ft_flags1,142b-140b,144f-143f ;
> > 	  .popsection ; .pushsection .altinstr_replacement,"ax" ;
> >       143: \newinstr1 ;
> >       144: .popsection ; ;
> > 
> >     141: .skip -(((144f-143f)-(141b-140b)) > 0) * ((144f-143f)-(141b-140b)),0x90 ;
> >     142: .pushsection .altinstructions,"a" ;
> > 	altinstr_entry 140b,143f,\ft_flags2,142b-140b,144f-143f ;
> > 	.popsection ;
> > 	.pushsection .altinstr_replacement,"ax" ;
> >     143: \newinstr2 ;
> >     144: .popsection ;
> >    .endm
> > 
> > The only label that is ambiguous is 140, however they all reference
> > the same spot, so that doesn't matter.
> > 
> > NOTE: obviously only @oldinstr may be an alternative; making @newinstr
> > an alternative would mean patching .altinstr_replacement which very
> > likely isn't what is intended, also the labels will be confused in
> > that case.
> > 
> 
> Reviewed-by: Nikolay Borisov <nik.borisov@...e.com>
> 
> Ps. I feel very "enlightened" knowing that GAS uses -1 to represent true ...

Ah, but only sometimes ;-)