Messages in this thread | | | From | Andy Lutomirski <> | Date | Mon, 30 Mar 2015 07:30:38 -0700 | Subject | Re: [PATCH] x86/asm/entry/64: better check for canonical address |
| |
On Mon, Mar 30, 2015 at 7:27 AM, Denys Vlasenko <dvlasenk@redhat.com> wrote: > On 03/26/2015 07:45 PM, Andy Lutomirski wrote: >> On Thu, Mar 26, 2015 at 5:42 AM, Denys Vlasenko <dvlasenk@redhat.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@redhat.com> >>> CC: Borislav Petkov <bp@alien8.de> >>> CC: x86@kernel.org >>> CC: linux-kernel@vger.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
| |