linux-kernel - Re: [PATCH v4 2/3] powerpc/modules: Don't try to restore r2 after a sibling call

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]

Date:   Thu, 16 Nov 2017 18:39:03 +0530
From:   "Naveen N. Rao" <naveen.n.rao@...ux.vnet.ibm.com>
To:     Josh Poimboeuf <jpoimboe@...hat.com>
Cc:     Balbir Singh <bsingharora@...il.com>,
        Kamalesh Babulal <kamalesh@...ux.vnet.ibm.com>,
        linux-kernel@...r.kernel.org, linuxppc-dev@...ts.ozlabs.org,
        live-patching@...r.kernel.org,
        Michael Ellerman <mpe@...erman.id.au>
Subject: Re: [PATCH v4 2/3] powerpc/modules: Don't try to restore r2 after a
 sibling call

Josh Poimboeuf wrote:
> On Wed, Nov 15, 2017 at 02:58:33PM +0530, Naveen N. Rao wrote:
>> > +int instr_is_link_branch(unsigned int instr)
>> > +{
>> > +	return (instr_is_branch_iform(instr) || instr_is_branch_bform(instr)) &&
>> > +	       (instr & BRANCH_SET_LINK);
>> > +}
>> > +
>> 
>> Nitpicking here, but since we're not considering the other branch forms,
>> perhaps this can be renamed to instr_is_link_relative_branch() (or maybe
>> instr_is_relative_branch_link()), just so we're clear :)
> 
> My understanding is that the absolute/relative bit isn't a "form", but
> rather a bit that can be set for either the b-form (conditional) or the
> i-form (unconditional).  And the above function isn't checking the
> absolute bit, so it isn't necessarily a relative branch.  Or did I miss
> something?

Ah, good point. I was coming from the fact that we are only considering 
the i-form and b-form branches and not the lr/ctr/tar based branches, 
which are always absolute branches, but can also set the link register.

Thinking about this more, aren't we only interested in relative branches
here (for relocations), so can we actually filter out the absolute 
branches? Something like this?

int instr_is_relative_branch_link(unsigned int instr)
{
	return ((instr_is_branch_iform(instr) || instr_is_branch_bform(instr)) &&
	       !(instr & BRANCH_ABSOLUTE) && (instr & BRANCH_SET_LINK));
}


- Naveen