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

Re: [Xen-devel] [PATCH v4 0/3] x86: modify_ldt improvement, test, and config option



On 07/27/2015 11:16 PM, Andy Lutomirski wrote:
On Mon, Jul 27, 2015 at 7:20 PM, Andy Lutomirski <luto@xxxxxxxxxxxxxx> wrote:
On Mon, Jul 27, 2015 at 9:18 AM, Boris Ostrovsky
<boris.ostrovsky@xxxxxxxxxx> wrote:
On 07/27/2015 11:53 AM, Andy Lutomirski wrote:
On Mon, Jul 27, 2015 at 8:36 AM, Boris Ostrovsky
<boris.ostrovsky@xxxxxxxxxx> wrote:
On 07/25/2015 01:36 AM, Andy Lutomirski wrote:
Here's v3.  It fixes the "dazed and confused" issue, I hope.  It's also
probably a good general attack surface reduction, and it replaces some
scary code with IMO less scary code.

Also, servers and embedded systems should probably turn off modify_ldt.
This makes that possible.

Xen people, can you take a look at this?

Willy and Kees: I left the config option alone.  The -tiny people will
like it, and we can always add a sysctl of some sort later.

Changes from v3:
    - Hopefully fixed Xen.

32b-on-32b fails in the same manner. (but non-zero LDT is taken care of)

    - Fixed 32-bit test case on 32-bit native kernel.

I am not sure I see what changed.
I misplaced the fix in the wrong git commit, so I failed to sent it.
Oops.

I just sent v4.1 of patch 3.  Can you try that?


I am hitting BUG() in Xen code (returning from a hypercall) when freeing LDT
in destroy_context(). Interestingly though when I run the test in the
debugger I get SIGILL (just like before) but no BUG().

Let me get back to you on that later today.


After forward-porting my virtio patches, I got this thing to run on
Xen.  After several tries, I got:

[   53.985707] ------------[ cut here ]------------
[   53.986314] kernel BUG at arch/x86/xen/enlighten.c:496!
[   53.986677] invalid opcode: 0000 [#1] SMP
[   53.986677] Modules linked in:
[   53.986677] CPU: 0 PID: 1400 Comm: bash Not tainted 4.2.0-rc4+ #4
[   53.986677] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996),
BIOS rel-1.7.5-0-ge51488c-20140602_164612-nilsson.home.kraxel.org
04/01/2014
[   53.986677] task: c2376180 ti: c0874000 task.ti: c0874000
[   53.986677] EIP: 0061:[<c10530f2>] EFLAGS: 00010282 CPU: 0
[   53.986677] EIP is at set_aliased_prot+0xb2/0xc0
[   53.986677] EAX: ffffffea EBX: cc3d1000 ECX: 0672e063 EDX: 80000000
[   53.986677] ESI: 00000000 EDI: 80000000 EBP: c0875e94 ESP: c0875e74
[   53.986677]  DS: 007b ES: 007b FS: 00d8 GS: 0033 SS: 0069
[   53.986677] CR0: 80050033 CR2: b77404d4 CR3: 020b6000 CR4: 00042660
[   53.986677] Stack:
[   53.986677]  80000000 0672e063 000021c0 cc3d1000 00000001 cc3d2000
00000b4a 00000200
[   53.986677]  c0875ea8 c105312d c2317940 c2373a80 00000000 c0875eb4
c1062310 c01861c0
[   53.986677]  c0875ec0 c1062735 c01861c0 c0875ed4 c10a764e c7007a00
c2373a80 00000000
[   53.986677] Call Trace:
[   53.986677]  [<c105312d>] xen_free_ldt+0x2d/0x40
[   53.986677]  [<c1062310>] free_ldt_struct.part.1+0x10/0x40
[   53.986677]  [<c1062735>] destroy_context+0x25/0x40
[   53.986677]  [<c10a764e>] __mmdrop+0x1e/0xc0
[   53.986677]  [<c10c9858>] finish_task_switch+0xd8/0x1a0
[   53.986677]  [<c1863736>] __schedule+0x316/0x950
[   53.986677]  [<c1863d96>] schedule+0x26/0x70
[   53.986677]  [<c10ac613>] do_wait+0x1b3/0x200
[   53.986677]  [<c10ac9d7>] SyS_waitpid+0x67/0xd0
[   53.986677]  [<c10aa820>] ? task_stopped_code+0x50/0x50
[   53.986677]  [<c186717a>] syscall_call+0x7/0x7
[   53.986677] Code: e8 c1 e3 0c 81 eb 00 00 00 40 39 5d ec 74 11 8b
4d e4 8b 55 e0 31 f6 e8 dd e0 fa ff 85 c0 75 0d 83 c4 14 5b 5e 5f 5d
c3 90 0f 0b <0f> 0b 0f 0b 8d 76 00 8d bc 27 00 00 00 00 85 d2 74 31 55
89 e5
[   53.986677] EIP: [<c10530f2>] set_aliased_prot+0xb2/0xc0 SS:ESP 0069:c0875e74
[   54.010069] ---[ end trace 89ac35b29c1c59bb ]---

Is that the error you're seeing?

If I change xen_free_ldt to:

static void xen_free_ldt(struct desc_struct *ldt, unsigned entries)
{
     const unsigned entries_per_page = PAGE_SIZE / LDT_ENTRY_SIZE;
     int i;

     vm_unmap_aliases();
     xen_mc_flush();

     for(i = 0; i < entries; i += entries_per_page)
         set_aliased_prot(ldt + i, PAGE_KERNEL);
}

then it works.  I don't know why this makes a difference.
(xen_mc_flush makes a little bit of sense to me.  vm_unmap_aliases
doesn't.)

That fix makes sense if there's some way that the vmalloc area we're
freeing has an extra alias somewhere, which is very much possible.  On
the other hand, I don't see how this happens without first doing an
MMUEXT_SET_LDT with an unexpectedly aliased address, and I would have
expected that to blow up and/or result in test case failures.

But I'm still confused, because it seems like Xen will never populate
the actual (hidden) LDT mapping unless the pages backing it are
unaliased and well-formed, which make me wonder why this stuff ever
worked.  Wouldn't LDT access with pre-existing vmalloc aliases result
in segfaults?

The semantics seem to be very odd.  xen_free_ldt with an aliased
address might fail (and OOPS), but actual access to the LDT with an
aliased address page faults.

Also, using kzalloc for everything fixes the problem, which suggests
that there really is something to my theory that the problem involves
unexpected aliases.

Yes, this is as far as I got as well (I didn't try unaliasing but now that you found it -- it does indeed work). I am not sure whether you are saying this (I think you do, implicitly, since you are replacing vzalloc with kzalloc), but the problem only happens when we have multi-page LDT.

And it is reproducible with a single CPU.


-boris


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.