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]

Date:	Mon, 30 Mar 2015 07:30:38 -0700
From:	Andy Lutomirski <luto@...capital.net>
To:	Denys Vlasenko <dvlasenk@...hat.com>
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 Mon, Mar 30, 2015 at 7:27 AM, Denys Vlasenko <dvlasenk@...hat.com> wrote:
> 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 :)
>

Will do, but not until later this week because I'm on vacation and I'm
allocating about ten minutes to using the computer :)  Or maybe Ingo
will beat me.

-- 
Andy Lutomirski
AMA Capital Management, LLC
--
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/