From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752698AbdKHQOn (ORCPT ); Wed, 8 Nov 2017 11:14:43 -0500 Received: from mx1.redhat.com ([209.132.183.28]:39470 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751944AbdKHQOm (ORCPT ); Wed, 8 Nov 2017 11:14:42 -0500 Subject: Re: [tip:x86/asm] x86/umip: Add emulation code for UMIP instructions To: pbonzini@redhat.com, hpa@zytor.com, bp@suse.de, vbabka@suse.cz, tony.luck@intel.com, fenghua.yu@intel.com, jpoimboe@redhat.com, bp@alien8.de, brgerst@gmail.com, corbet@lwn.net, ravi.v.shankar@intel.com, ricardo.neri-calderon@linux.intel.com, torvalds@linux-foundation.org, cmetcalf@mellanox.com, akpm@linux-foundation.org, paul.gortmaker@windriver.com, mhiramat@kernel.org, luto@kernel.org, dave.hansen@linux.intel.com, ray.huang@amd.com, mst@redhat.com, linux-kernel@vger.kernel.org, peterz@infradead.org, slaoub@gmail.com, mingo@kernel.org, jslaby@suse.cz, shuah@kernel.org, tglx@linutronix.de, linux-tip-commits@vger.kernel.org References: <1509935277-22138-8-git-send-email-ricardo.neri-calderon@linux.intel.com> From: Denys Vlasenko Message-ID: Date: Wed, 8 Nov 2017 17:14:33 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.4.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.25]); Wed, 08 Nov 2017 16:14:42 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 11/08/2017 12:00 PM, tip-bot for Ricardo Neri wrote: > Commit-ID: 1e5db223696afa55e6a038fac638f759e1fdcc01 > Gitweb: https://git.kernel.org/tip/1e5db223696afa55e6a038fac638f759e1fdcc01 > Author: Ricardo Neri > AuthorDate: Sun, 5 Nov 2017 18:27:52 -0800 > Committer: Ingo Molnar > CommitDate: Wed, 8 Nov 2017 11:16:22 +0100 > > x86/umip: Add emulation code for UMIP instructions > > The feature User-Mode Instruction Prevention present in recent Intel > processor prevents a group of instructions (sgdt, sidt, sldt, smsw, and > str) from being executed with CPL > 0. Otherwise, a general protection > fault is issued. This was arguably an oversight on Intel's part - these insns should have been protected from the start, as they leak a tiny bit of kernel data. > Rather than relaying to the user space the general protection fault caused > by the UMIP-protected instructions (in the form of a SIGSEGV signal), it > can be trapped and the instruction emulated to provide a dummy result. > This allows to both conserve the current kernel behavior and not reveal the > system resources that UMIP intends to protect (i.e., the locations of the > global descriptor and interrupt descriptor tables, the segment selectors of > the local descriptor table, the value of the task state register and the > contents of the CR0 register). > > This emulation is needed because certain applications (e.g., WineHQ and > DOSEMU2) rely on this subset of instructions to function. I'm surprised. What in the world they need those insns for? Wine uses sidt like this, to emulate "mov from r/m to reg" insns: static LDT_ENTRY idt[256]; ... case 0x8a: /* mov Eb, Gb */ case 0x8b: /* mov Ev, Gv */ { BYTE *data = INSTR_GetOperandAddr(context, instr + 1, long_addr, segprefix, &len); unsigned int data_size = (*instr == 0x8b) ? (long_op ? 4 : 2) : 1; struct idtr idtr = get_idtr(); <=============================== HERE unsigned int offset = data - idtr.base; if (offset <= idtr.limit + 1 - data_size) { idt[1].LimitLow = 0x100; /* FIXME */ idt[2].LimitLow = 0x11E; /* FIXME */ idt[3].LimitLow = 0x500; /* FIXME */ switch (*instr) { case 0x8a: store_reg_byte( context, instr[1], (BYTE *)idt + offset ); break; case 0x8b: store_reg_word( context, instr[1], (BYTE *)idt + offset, long_op ); break; } context->Eip += prefixlen + len + 1; return ExceptionContinueExecution; } break; /* Unable to emulate it */ } Looks baffling, to say the least... this supports someone who reads IDT bytes via those insns, and they need to ensure that the values read from idt[1/2/3].LimitLow are as expected. That's it? Pity git history doesn't go far enough in the past, and comments are not informative as well... I did not find smsw or sgdt in Wine git tree. I did not find smsw, sidt or sgdt in dosemu2-devel git tree. Can we avoid maintain emulation of these isns, by asking Wine to remove their use instead?