linux-kernel - Re: [PATCH] x86/asm/entry/64: better check for canonical address

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: <55195D3E.4060608@redhat.com>
Date:	Mon, 30 Mar 2015 16:27:10 +0200
From:	Denys Vlasenko <dvlasenk@...hat.com>
To:	Andy Lutomirski <luto@...capital.net>
CC:	Borislav Petkov <bp@...en8.de>, X86 ML <x86@...nel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] x86/asm/entry/64: better check for canonical address

On 03/26/2015 07:45 PM, Andy Lutomirski wrote:
> On Thu, Mar 26, 2015 at 5:42 AM, Denys Vlasenko <dvlasenk@...hat.com> wrote:
>> This change makes the check exact (no more false positives
>> on kernel addresses).
>>
>> It isn't really important to be fully correct here -
>> almost all addresses we'll ever see will be userspace ones,
>> but OTOH it looks to be cheap enough:
>> the new code uses two more ALU ops but preserves %rcx,
>> allowing to not reload it from pt_regs->cx again.
>> On disassembly level, the changes are:
>>
>> cmp %rcx,0x80(%rsp) -> mov 0x80(%rsp),%r11; cmp %rcx,%r11
>> shr $0x2f,%rcx      -> shl $0x10,%rcx; sar $0x10,%rcx; cmp %rcx,%r11
>> mov 0x58(%rsp),%rcx -> (eliminated)
>>
>> Signed-off-by: Denys Vlasenko <dvlasenk@...hat.com>
>> CC: Borislav Petkov <bp@...en8.de>
>> CC: x86@...nel.org
>> CC: linux-kernel@...r.kernel.org
>> ---
>>
>> Andy, I'd undecided myself on the merits of doing this.
>> If you like it, feel free to take it in your tree.
>> I trimmed CC list to not bother too many people with this trivial
>> and quite possibly "useless churn"-class change.
> 
> I suspect that the two added ALU ops are free for all practical
> purposes, and the performance of this path isn't *that* critical.
> 
> If anyone is running with vsyscall=native because they need the
> performance, then this would be a big win.  Otherwise I don't have a
> real preference.  Anyone else have any thoughts here?
> 
> Let me just run through the math quickly to make sure I believe all the numbers:
> 
> Canonical addresses either start with 17 zeros or 17 ones.
> 
> In the old code, we checked that the top (64-47) = 17 bits were all
> zero.  We did this by shifting right by 47 bits and making sure that
> nothing was left.
> 
> In the new code, we're shifting left by (64 - 48) = 16 bits and then
> signed shifting right by the same amount, this propagating the 17th
> highest bit to all positions to its left.  If we get the same value we
> started with, then we're good to go.
> 
> So it looks okay to me.


So please take it into your tree :)

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/