linux-kernel - Re: [PATCH 3/5] MIPS: Only change $28 to thread

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: <9579f0c6-9f67-cf1d-94c1-34d14f38afd6@imgtec.com>
Date:   Mon, 5 Dec 2016 16:52:54 +0000
From:   Matt Redfearn <matt.redfearn@...tec.com>
To:     "Maciej W. Rozycki" <macro@...tec.com>
CC:     Ralf Baechle <ralf@...ux-mips.org>, <linux-mips@...ux-mips.org>,
        "Jason A . Donenfeld" <Jason@...c4.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        <linux-kernel@...r.kernel.org>,
        James Hogan <james.hogan@...tec.com>,
        Paul Burton <paul.burton@...tec.com>
Subject: Re: [PATCH 3/5] MIPS: Only change $28 to thread_info if coming from
 user mode


Hi Maciej,

On 05/12/16 16:20, Maciej W. Rozycki wrote:
> On Fri, 2 Dec 2016, Matt Redfearn wrote:
>
>> diff --git a/arch/mips/include/asm/stackframe.h b/arch/mips/include/asm/stackframe.h
>> index eebf39549606..5782fa3d63be 100644
>> --- a/arch/mips/include/asm/stackframe.h
>> +++ b/arch/mips/include/asm/stackframe.h
>> @@ -216,12 +216,22 @@
>>   		LONG_S	$25, PT_R25(sp)
>>   		LONG_S	$28, PT_R28(sp)
>>   		LONG_S	$31, PT_R31(sp)
>> +
>> +		/* Set thread_info if we're coming from user mode */
>> +		.set	reorder
>> +		mfc0	k0, CP0_STATUS
>> +		sll	k0, 3		/* extract cu0 bit */
>> +		.set	noreorder
>> +		bltz	k0, 9f
>> +		 nop
>   This code is already `.set reorder', although a badly applied CONFIG_EVA
> change made things slightly less obvious.  So why do you need this `.set
> reorder' in the first place, and then why do you switch code that follows
> to `.set noreorder'?

Ah yes, I missed the .set reorder above the EVA ifdef and just included 
the .set reorder as the similar snippet here:
http://lxr.free-electrons.com/source/arch/mips/include/asm/stackframe.h#L149

I'll revise that in V2.

Thanks,
Matt

>
>   Overall I think all <asm/stackframe.h> code should be using the (default)
> `.set reorder' mode, perhaps forced explicitly in case these macros are
> pasted into `.set noreorder' code, to make it easier to avoid subtle data
> dependency bugs, and also to make R6 porting easier.  Except maybe for the
> RFE sequence, for readability's sake, although even there GAS will do the
> right thing.  Surely the BLTZ/MOVE piece does not have to be `.set
> noreorder' as GAS will schedule that delay slot automatically if allowed
> to.
>
>    Maciej