From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754272AbbBZTSX (ORCPT ); Thu, 26 Feb 2015 14:18:23 -0500 Received: from mail-la0-f43.google.com ([209.85.215.43]:34005 "EHLO mail-la0-f43.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753817AbbBZTSV (ORCPT ); Thu, 26 Feb 2015 14:18:21 -0500 MIME-Version: 1.0 In-Reply-To: References: From: Andy Lutomirski Date: Thu, 26 Feb 2015 11:17:59 -0800 Message-ID: Subject: Re: [PATCH 3.2 055/152] x86_64, switch_to(): Load TLS descriptors before switching DS and ES To: Brian Gerst Cc: Denys Vlasenko , Ben Hutchings , Linux Kernel Mailing List , stable , Andrew Morton , Ingo Molnar , Andi Kleen , Linus Torvalds Content-Type: multipart/mixed; boundary=001a11c3397e5e16da0510029db2 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org --001a11c3397e5e16da0510029db2 Content-Type: text/plain; charset=UTF-8 On Thu, Feb 26, 2015 at 8:28 AM, Brian Gerst wrote: > On Thu, Feb 26, 2015 at 10:32 AM, Andy Lutomirski wrote: >> On Tue, Feb 24, 2015 at 7:23 PM, Brian Gerst wrote: >>> On Tue, Feb 24, 2015 at 3:08 PM, Denys Vlasenko >>> wrote: >>>> On Tue, Feb 24, 2015 at 9:02 PM, Andy Lutomirski wrote: >>>>>> This currently fails in 32-bit kernels (at least in qemu): >>>>>> >>>>>> / # ./es_test >>>>>> Allocated GDT index 7 >>>>>> [FAIL] ES changed from 0x3b to 0x7b >>>>>> [FAIL] ES was corrupted 1000/1000 times >>>>>> / # uname -a >>>>>> Linux (none) 4.0.0-rc1 #1 SMP Tue Feb 24 16:41:58 CET 2015 i686 GNU/Linux >>>>> >>>>> Want to send a patch? I'll get it in a few days if no one beats me. >>>> >>>> I have no patch, sorry (in fact, I failed to find where is the relevant >>>> 32-bit counterpart). >>>> >>>> It's just security people asked me to backport this and I wondered >>>> maybe I should wait a bit on this one, since fix for 32-bit ought >>>> to appear as well. >>> >>> For 32-bit kernel, userspace DS and ES are saved at syscall/interrupt >>> entry time and reloaded on exit, unlike in 64-bit where they are saved >>> and loaded at context switch time. Therefore 32-bit is not affected >>> by the issue this patch addresses. >>> >>> It looks to me though, that the ES test program doesn't actually test >>> what the patch fixes - the segment attributes, like the base address. >>> It tests just the selector, which shouldn't change across a kernel >>> entry (with a few exceptions, like signals). If the test is failing, >>> then it is a different issue from what this patch addresses. >> >> It tests it indirectly. The 64-bit code sets the selector to zero if >> it fails to reload it. Testing the ES base is awkward because it >> can't be done in 64-bit code at all. > > I figured out why Denys got the failure. usleep() makes a syscall via > sysenter. The sysenter path saves es/ds, but does not restore them > before sysexit like the int80/iret path would. That leaves them as > USER_DS that the kernel loaded for itself. I believe this was an > intentional optimization, assuming the vdso would only be called from > programs conforming to the ELF ABI. Makes sense. The attached variant passes, so I think we're fine. --Andy --001a11c3397e5e16da0510029db2 Content-Type: text/x-csrc; charset=US-ASCII; name="estest.c" Content-Disposition: attachment; filename="estest.c" Content-Transfer-Encoding: base64 X-Attachment-Id: f_i6mj9tn10 LyoKICogQ29weXJpZ2h0IChjKSAyMDE0LTIwMTUgQW5keSBMdXRvbWlyc2tpCiAqIEdQTCB2Mgog Ki8KCiNpbmNsdWRlIDxzdGRpby5oPgojaW5jbHVkZSA8dW5pc3RkLmg+CiNpbmNsdWRlIDx0aW1l Lmg+CiNpbmNsdWRlIDxlcnIuaD4KI2luY2x1ZGUgPGFzbS9sZHQuaD4KI2luY2x1ZGUgPHN5cy9z eXNjYWxsLmg+CgpzdGF0aWMgdW5zaWduZWQgc2hvcnQgR0RUMyhpbnQgaWR4KQp7CglyZXR1cm4g KGlkeCA8PCAzKSB8IDM7Cn0KCnN0YXRpYyBpbnQgY3JlYXRlX3RscyhpbnQgaWR4LCB1bnNpZ25l ZCBpbnQgYmFzZSkKewoJc3RydWN0IHVzZXJfZGVzYyBkZXNjID0gewoJCS5lbnRyeV9udW1iZXIg ICAgPSBpZHgsCgkJLmJhc2VfYWRkciAgICAgICA9IGJhc2UsCgkJLmxpbWl0ICAgICAgICAgICA9 IDB4ZmZmZmYsCgkJLnNlZ18zMmJpdCAgICAgICA9IDEsCgkJLmNvbnRlbnRzICAgICAgICA9IDAs IC8qIERhdGEsIGdyb3ctdXAgKi8KCQkucmVhZF9leGVjX29ubHkgID0gMCwKCQkubGltaXRfaW5f cGFnZXMgID0gMSwKCQkuc2VnX25vdF9wcmVzZW50ID0gMCwKCQkudXNlYWJsZSAgICAgICAgID0g MCwKCX07CgoJaWYgKHN5c2NhbGwoU1lTX3NldF90aHJlYWRfYXJlYSwgJmRlc2MpICE9IDApCgkJ ZXJyKDEsICJzZXRfdGhyZWFkX2FyZWEiKTsKCglyZXR1cm4gZGVzYy5lbnRyeV9udW1iZXI7Cn0K CmludCBtYWluKCkKewoJaW50IGlkeCA9IGNyZWF0ZV90bHMoLTEsIDApOwoJcHJpbnRmKCJBbGxv Y2F0ZWQgR0RUIGluZGV4ICVkXG4iLCBpZHgpOwoKCXVuc2lnbmVkIHNob3J0IG9yaWdfZXM7Cglh c20gdm9sYXRpbGUgKCJtb3YgJSVlcywlMCIgOiAiPXJtIiAob3JpZ19lcykpOwoKCWludCBlcnJv cnMgPSAwOwoJaW50IHRvdGFsID0gMTAwMDsKCWZvciAoaW50IGkgPSAwOyBpIDwgdG90YWw7IGkr KykgewoJCXN0cnVjdCB0aW1lc3BlYyByZXEgPSB7CgkJCS50dl9zZWMgPSAwLAoJCQkudHZfbnNl YyA9IDEwMDAwMCwKCQl9OwoJCWludCByZXQ7CgoJCWFzbSB2b2xhdGlsZSAoIm1vdiAlMCwlJWVz IiA6IDogInJtIiAoR0RUMyhpZHgpKSk7CgoJCS8qCgkJICogRm9yY2UgcmVzY2hlZHVsaW5nLiAg T24gMzItYml0IGtlcm5lbHMsIGZhc3Qgc3lzY2FsbHMKCQkgKiBkZXN0cm95IERTIGFuZCBFUywg c28gZm9yY2UgaW50IDgwLgoJCSAqLwoJCWFzbSB2b2xhdGlsZSAoImludCAkMHg4MCIKCQkJICAg ICAgOiAiPWEiIChyZXQpCgkJCSAgICAgIDogImEiIChTWVNfbmFub3NsZWVwKSwgImIiICgmcmVx KSwKCQkJCSJjIiAoMCkpOwoKCQl1bnNpZ25lZCBzaG9ydCBlczsKCQlhc20gdm9sYXRpbGUgKCJt b3YgJSVlcywlMCIgOiAiPXJtIiAoZXMpKTsKCQlhc20gdm9sYXRpbGUgKCJtb3YgJTAsJSVlcyIg OiA6ICJybSIgKG9yaWdfZXMpKTsKCQlpZiAoZXMgIT0gR0RUMyhpZHgpKSB7CgkJCWlmIChlcnJv cnMgPT0gMCkKCQkJCXByaW50ZigiW0ZBSUxdXHRFUyBjaGFuZ2VkIGZyb20gMHglaHggdG8gMHgl aHhcbiIsCgkJCQkgICAgICAgR0RUMyhpZHgpLCBlcyk7CgkJCWVycm9ycysrOwoJCX0KCX0KCglp ZiAoZXJyb3JzKSB7CgkJcHJpbnRmKCJbRkFJTF1cdEVTIHdhcyBjb3JydXB0ZWQgJWQvJWQgdGlt ZXNcbiIsIGVycm9ycywgdG90YWwpOwoJCXJldHVybiAxOwoJfSBlbHNlIHsKCQlwcmludGYoIltP S11cdEVTIHdhcyBwcmVzZXJ2ZWRcbiIpOwoJCXJldHVybiAwOwoJfQp9Cg== --001a11c3397e5e16da0510029db2--