lkml.org 
[lkml]   [2015]   [Jul]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
From
Date
SubjectRe: [PATCH v4 0/3] x86: modify_ldt improvement, test, and config option
On Mon, Jul 27, 2015 at 9:18 AM, Boris Ostrovsky
<boris.ostrovsky@oracle.com> wrote:
> On 07/27/2015 11:53 AM, Andy Lutomirski wrote:
>>
>> On Mon, Jul 27, 2015 at 8:36 AM, Boris Ostrovsky
>> <boris.ostrovsky@oracle.com> 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.)

It's *possible* that there's some but in my code that causes a CPU to
retain a reference to a stale LDT, but I don't see it. Hmm. Is it
possible that, when a process exits, we kill the mm without
synchronously unlazying it everywhere else? Seems a bit hard to
imagine to me -- I don't see why this wouldn't blow up when the pgt
went away.

My best guess is that there's a silly race in which one CPU frees and
LDT before the other CPU flushes its hypercalls. But I don't really
believe this, because I got this trace:

[ 14.257546] Free LDT cb912000: CPU0 cb923000 CPU1 cb923000
[OK] All 5 iterations succeeded
root@(none):/# [ 15.824723] Free LDT cb923000: CPU0 (null) CPU1 (null)
[ 15.827404] ------------[ cut here ]------------
[ 15.828349] kernel BUG at arch/x86/xen/enlighten.c:497!
[ 15.828349] invalid opcode: 0000 [#1] SMP

with this patch applied:

@@ -537,7 +542,9 @@ static void xen_set_ldt(const void *addr, unsigned entries)

MULTI_mmuext_op(mcs.mc, op, 1, NULL, DOMID_SELF);

- xen_mc_issue(PARAVIRT_LAZY_CPU);
+ xen_mc_flush();
+
+ this_cpu_write(cpu_ldt, addr);
}

so both CPUs on my VM have definitely zeroed their LDTs before the
failing hypercall.

Hmm. Looking at the hypervisor code, I don't see why setting the LDT
to NULL is handled correctly. Am I missing something?
--Andy


\
 
 \ /
  Last update: 2015-07-28 04:41    [W:0.188 / U:0.036 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site