Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752387AbdLLJAO (ORCPT ); Tue, 12 Dec 2017 04:00:14 -0500 Received: from mga01.intel.com ([192.55.52.88]:26258 "EHLO mga01.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751622AbdLLJAN (ORCPT ); Tue, 12 Dec 2017 04:00:13 -0500 X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.45,394,1508828400"; d="scan'208";a="157991810" Subject: Re: Re: WARNING in x86_emulate_insn To: Paolo Bonzini , Tianyu Lan , Jim Mattson References: <001a1143d526c5b1aa055f9d604c@google.com> Cc: Wanpeng Li , Dmitry Vyukov , syzbot , "H. Peter Anvin" , kvm , "linux-kernel@vger.kernel.org" , Ingo Molnar , Radim Krcmar , syzkaller-bugs@googlegroups.com, Thomas Gleixner , the arch/x86 maintainers From: Lan Tianyu Message-ID: Date: Tue, 12 Dec 2017 16:50:11 +0800 User-Agent: Mozilla/5.0 (X11; Linux i686 on x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.3.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2091 Lines: 62 On 2017年12月12日 06:45, Paolo Bonzini wrote: > On 08/12/2017 09:28, Tianyu Lan wrote: >> I find this is pop instruction emulation issue. According "SDM VOL2, >> chapter INSTRUCTION >> SET REFERENCE. POP—Pop a Value from the Stack" >> >> Protected Mode Exceptions >> #GP(0) If attempt is made to load SS register with NULL segment selector. > > This is not what the testcase is testing; this is already covered by > __load_segment_descriptor: > > if (null_selector) { > if (seg == VCPU_SREG_CS || seg == VCPU_SREG_TR) > goto exception; > > if (seg == VCPU_SREG_SS) { > if (ctxt->mode != X86EMUL_MODE_PROT64 || rpl != cpl) > goto exception; > ... > } Yes, __load_segment_descriptor() does such check. I find em_pop doesn't load SS segment. SS isn't loaded before calling em_pop in the test case. Should this be fixed? > > Is there a path that can return X86EMUL_PROPAGATE_FAULT without setting > ctxt->exception.vector and/or without going through emulate_exception? > > I don't think it's possible to write a test in kvm-unit-tests, because the > state has "impossible" segment descriptor cache contents. Sent out a fix patch for the issue. Please have a look. Thanks. https://marc.info/?l=kvm&m=151306208214733&w=2 > > Paolo > >> This test case hits it but current code doesn't check such case. >> The following patch can fix the issue. >> >> diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c >> index abe74f7..e2ac5cc 100644 >> --- a/arch/x86/kvm/emulate.c >> +++ b/arch/x86/kvm/emulate.c >> @@ -1844,6 +1844,9 @@ static int emulate_pop(struct x86_emulate_ctxt *ctxt, >> int rc; >> struct segmented_address addr; >> >> + if ( !get_segment_selector(ctxt, VCPU_SREG_SS)) >> + return emulate_gp(ctxt, 0); >> + >> addr.ea = reg_read(ctxt, VCPU_REGS_RSP) & stack_mask(ctxt); >> addr.seg = VCPU_SREG_SS; >> rc = segmented_read(ctxt, addr, dest, len); > -- Best regards Tianyu Lan