Re: Patch "x86-64, espfix: Don't leak bits 31:16 of %esp returning to 16-bit stack" has been added to the 3.15-stable tree

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 07/14/2014 10:40 PM, Boris Ostrovsky wrote:
On 07/14/2014 10:30 PM, Andrew Lutomirski wrote:
On Mon, Jul 14, 2014 at 7:21 PM, Boris Ostrovsky
<boris.ostrovsky@xxxxxxxxxx> wrote:
On 07/14/2014 06:47 PM, H. Peter Anvin wrote:
On 07/14/2014 02:43 PM, Andrew Lutomirski wrote:
That patch is running through my build tests as we speak. I expect to
push it to tip:x86/urgent in about an hour.
I think that espfix64 is completely broken on Xen, regardless of the
pud vs pmd issue :(  See:


http://lkml.kernel.org/g/CALCETrWG-dQL8ipJ8cO3wfbYKA=mAv3CS4-1JFwmBXF3pUbAwg@xxxxxxxxxxxxxx

This is exactly the problem that the patch is fixing:
     paravirt_alloc_pte(&init_mm, __pa(stack_page) >> PAGE_SHIFT)
that we are removing is trying claim that stack_page is going to be the PTE
page (i.e. the last level). So the hypervisor will mark it read-only.
I don't follow.  The espfix64 stack is read-only by design.

Isn't 'movq %rax,(0*8)(%rdi)' from http://article.gmane.org/gmane.linux.kernel/1746680 trying to write stack_page?


Any time someone writes to it the hypervisor presumes that this is a PTE update and will perform various sanity checks before allowing the update to succeed. Obviously writing random data (from PTE format perspective) will
not be allowed and a fault will be returned to the guest.

You can easily simulate this by, for example, allocating stack_page with __GFP_ZERO and writing -1 to espfix_waddr at the end of init_espfix_ap().


And it is also completely unclear to me if it is actually necessary.
Again, I would like to know how the Xen IRET pvop actually handles a
16-bit stack segment.  If it restores all of RSP then espfix isn't
necessary.

Last time I looked at this I thought it was not necessary since the iret
handler in the hypervisor copies saved RSP. I may be wrong though.
How?  This is a CPU misdesign we're talking about.

I was referring to IRET hypecall : http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=xen/arch/x86/x86_64/traps.c;h=650c33d3da7eb8a76d26a1e4a934b32e67587a40;hb=HEAD

Sorry, didn't include the line number:

http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=xen/arch/x86/x86_64/traps.c;h=650c33d3da7eb8a76d26a1e4a934b32e67587a40;hb=HEAD#l289


There may be other paths, but that's the one that I had in mind.

-boris


--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel]     [Kernel Development Newbies]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Hiking]     [Linux Kernel]     [Linux SCSI]