From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andy Lutomirski Subject: Re: [PATCH v4 0/3] x86: modify_ldt improvement, test, and config option Date: Mon, 27 Jul 2015 20:23:01 -0700 Message-ID: References: <55B64FEA.70204@oracle.com> <55B659EC.5030009@oracle.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary=001a11c264be067025051be6fea5 Return-path: In-Reply-To: List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Boris Ostrovsky Cc: "security@kernel.org" , Jan Beulich , Peter Zijlstra , Andrew Cooper , X86 ML , "linux-kernel@vger.kernel.org" , Steven Rostedt , xen-devel , Borislav Petkov , Andy Lutomirski , Sasha Levin List-Id: xen-devel@lists.xenproject.org --001a11c264be067025051be6fea5 Content-Type: text/plain; charset=UTF-8 On Mon, Jul 27, 2015 at 8:16 PM, Andy Lutomirski wrote: > On Mon, Jul 27, 2015 at 7:20 PM, Andy Lutomirski wrote: >> On Mon, Jul 27, 2015 at 9:18 AM, Boris Ostrovsky >> wrote: >>> On 07/27/2015 11:53 AM, Andy Lutomirski wrote: >>>> >>>> On Mon, Jul 27, 2015 at 8:36 AM, Boris Ostrovsky >>>> 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:[] 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] [] xen_free_ldt+0x2d/0x40 >> [ 53.986677] [] free_ldt_struct.part.1+0x10/0x40 >> [ 53.986677] [] destroy_context+0x25/0x40 >> [ 53.986677] [] __mmdrop+0x1e/0xc0 >> [ 53.986677] [] finish_task_switch+0xd8/0x1a0 >> [ 53.986677] [] __schedule+0x316/0x950 >> [ 53.986677] [] schedule+0x26/0x70 >> [ 53.986677] [] do_wait+0x1b3/0x200 >> [ 53.986677] [] SyS_waitpid+0x67/0xd0 >> [ 53.986677] [] ? task_stopped_code+0x50/0x50 >> [ 53.986677] [] 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: [] 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. The attachment fixes the problem for me, and I could easily believe that the attachment is correct. I'd like to know why the code appears to work without the xen_alloc_ldt change in there, though. --Andy --001a11c264be067025051be6fea5 Content-Type: text/x-diff; charset=US-ASCII; name="xen.patch" Content-Disposition: attachment; filename="xen.patch" Content-Transfer-Encoding: base64 X-Attachment-Id: f_icms39oi0 Y29tbWl0IGI0NzcyY2Y4NDlmMDVkNWNlYWIwNzliNGU4ODQ5N2RkMWI5OTBhY2QKQXV0aG9yOiBB bmR5IEx1dG9taXJza2kgPGx1dG9Aa2VybmVsLm9yZz4KRGF0ZTogICBNb24gSnVsIDI3IDIwOjIw OjU1IDIwMTUgLTA3MDAKCiAgICB4ODYveGVuOiBVbm1hcCBhbGlhc2VzIGluIHhlbl9hbGxvY19s ZHQgYW5kIHhlbl9mcmVlX2xkdAogICAgCiAgICBUaGUgeGVuX2ZyZWVfbGR0IGNoYW5nZSBmaXhl cyBhbiBPT1BTIGluIHRoZSBuZXcgbW9kaWZ5X2xkdAogICAgaW1wbGVtZW50YXRpb24uICBJIHRo aW5rIHRoZSB4ZW5fYWxsb2NfbGR0IGNoYW5nZSBzaG91bGQgYmUKICAgIG5lY2Vzc2FyeSwgdG9v LCBidXQgSSBjYW4ndCBzZWVtIHRvIHRyaWdnZXIgYW55IGZhaWx1cmVzIHdpdGhvdXQgaXQsCiAg ICB3aGljaCBJIGZpbmQgc3VycHJpc2luZy4KICAgIAogICAgQ2M6IHN0YWJsZUB2Z2VyLmtlcm5l bC5vcmcKICAgIFNpZ25lZC1vZmYtYnk6IEFuZHkgTHV0b21pcnNraSA8bHV0b0BrZXJuZWwub3Jn PgoKZGlmZiAtLWdpdCBhL2FyY2gveDg2L3hlbi9lbmxpZ2h0ZW4uYyBiL2FyY2gveDg2L3hlbi9l bmxpZ2h0ZW4uYwppbmRleCAwYjk1YzliODI4M2YuLjEwMGEyZTIyOTRhZiAxMDA2NDQKLS0tIGEv YXJjaC94ODYveGVuL2VubGlnaHRlbi5jCisrKyBiL2FyY2gveDg2L3hlbi9lbmxpZ2h0ZW4uYwpA QCAtMzIsNiArMzIsNyBAQAogI2luY2x1ZGUgPGxpbnV4L2dmcC5oPgogI2luY2x1ZGUgPGxpbnV4 L21lbWJsb2NrLmg+CiAjaW5jbHVkZSA8bGludXgvZWRkLmg+CisjaW5jbHVkZSA8bGludXgvdm1h bGxvYy5oPgogCiAjaW5jbHVkZSA8eGVuL3hlbi5oPgogI2luY2x1ZGUgPHhlbi9ldmVudHMuaD4K QEAgLTUxMiw2ICs1MTMsMTAgQEAgc3RhdGljIHZvaWQgeGVuX2FsbG9jX2xkdChzdHJ1Y3QgZGVz Y19zdHJ1Y3QgKmxkdCwgdW5zaWduZWQgZW50cmllcykKIAogCWZvcihpID0gMDsgaSA8IGVudHJp ZXM7IGkgKz0gZW50cmllc19wZXJfcGFnZSkKIAkJc2V0X2FsaWFzZWRfcHJvdChsZHQgKyBpLCBQ QUdFX0tFUk5FTF9STyk7CisKKwkvKiBJZiB0aGVyZSBhcmUgc3RyYXkgYWxpYXNlcywgdGhlIExE VCB3b24ndCB3b3JrLiAqLworCWlmIChpc192bWFsbG9jX2FkZHIobGR0KSkKKwkJdm1fdW5tYXBf YWxpYXNlcygpOwogfQogCiBzdGF0aWMgdm9pZCB4ZW5fZnJlZV9sZHQoc3RydWN0IGRlc2Nfc3Ry dWN0ICpsZHQsIHVuc2lnbmVkIGVudHJpZXMpCkBAIC01MTksNiArNTI0LDEzIEBAIHN0YXRpYyB2 b2lkIHhlbl9mcmVlX2xkdChzdHJ1Y3QgZGVzY19zdHJ1Y3QgKmxkdCwgdW5zaWduZWQgZW50cmll cykKIAljb25zdCB1bnNpZ25lZCBlbnRyaWVzX3Blcl9wYWdlID0gUEFHRV9TSVpFIC8gTERUX0VO VFJZX1NJWkU7CiAJaW50IGk7CiAKKwkvKgorCSAqIElmIHRoZXJlIGFyZSBzdHJheSBhbGlhc2Vz LCBoeXBlcnZpc29yIHdpbGwgZmFpbCB0aGUgaHlwZXJjYWxscywKKwkgKiBjYXVzaW5nIHVzIHRv IE9PUFMuCisJICovCisJaWYgKGlzX3ZtYWxsb2NfYWRkcihsZHQpKQorCQl2bV91bm1hcF9hbGlh c2VzKCk7CisKIAlmb3IoaSA9IDA7IGkgPCBlbnRyaWVzOyBpICs9IGVudHJpZXNfcGVyX3BhZ2Up CiAJCXNldF9hbGlhc2VkX3Byb3QobGR0ICsgaSwgUEFHRV9LRVJORUwpOwogfQpkaWZmIC0tZ2l0 IGEvdG9vbHMvdGVzdGluZy9zZWxmdGVzdHMveDg2L2xkdF9nZHQuYyBiL3Rvb2xzL3Rlc3Rpbmcv c2VsZnRlc3RzL3g4Ni9sZHRfZ2R0LmMKaW5kZXggYzI3YWRmYzlhZTcyLi5mYmE1YmMxMzNhYTIg MTAwNjQ0Ci0tLSBhL3Rvb2xzL3Rlc3Rpbmcvc2VsZnRlc3RzL3g4Ni9sZHRfZ2R0LmMKKysrIGIv dG9vbHMvdGVzdGluZy9zZWxmdGVzdHMveDg2L2xkdF9nZHQuYwpAQCAtMjA0LDYgKzIwNCwxMCBA QCBzdGF0aWMgdm9pZCBkb19zaW1wbGVfdGVzdHModm9pZCkKIAl9OwogCWluc3RhbGxfdmFsaWQo JmRlc2MsIEFSX0RQTDMgfCBBUl9UWVBFX1hSQ09ERSB8IEFSX1MgfCBBUl9QIHwgQVJfREIpOwog CisJZGVzYy5lbnRyeV9udW1iZXIgPSA4MTkxOworCWluc3RhbGxfdmFsaWQoJmRlc2MsIEFSX0RQ TDMgfCBBUl9UWVBFX1hSQ09ERSB8IEFSX1MgfCBBUl9QIHwgQVJfREIpOworCWRlc2MuZW50cnlf bnVtYmVyID0gMDsKKwogCWRlc2MubGltaXRfaW5fcGFnZXMgPSAxOwogCWluc3RhbGxfdmFsaWQo JmRlc2MsIEFSX0RQTDMgfCBBUl9UWVBFX1hSQ09ERSB8CiAJCSAgICAgIEFSX1MgfCBBUl9QIHwg QVJfREIgfCBBUl9HKTsK --001a11c264be067025051be6fea5 Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel --001a11c264be067025051be6fea5--