From mboxrd@z Thu Jan 1 00:00:00 1970 From: Avi Kivity Subject: Re: [PATCH 5/8] KVM: Check IOPL level during io instruction emulation. Date: Tue, 09 Feb 2010 16:51:12 +0200 Message-ID: <4B717660.60506@redhat.com> References: <1265724844-11112-1-git-send-email-gleb@redhat.com> <1265724844-11112-6-git-send-email-gleb@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: mtosatti@redhat.com, kvm@vger.kernel.org To: Gleb Natapov Return-path: Received: from mx1.redhat.com ([209.132.183.28]:32696 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754036Ab0BIOvO (ORCPT ); Tue, 9 Feb 2010 09:51:14 -0500 Received: from int-mx05.intmail.prod.int.phx2.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.18]) by mx1.redhat.com (8.13.8/8.13.8) with ESMTP id o19EpDMD032247 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Tue, 9 Feb 2010 09:51:13 -0500 Received: from cleopatra.tlv.redhat.com (cleopatra.tlv.redhat.com [10.35.255.11]) by int-mx05.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id o19EpCqG021795 for ; Tue, 9 Feb 2010 09:51:13 -0500 In-Reply-To: <1265724844-11112-6-git-send-email-gleb@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: On 02/09/2010 04:14 PM, Gleb Natapov wrote: > Make emulator check that vcpu is allowed to execute IN, INS, OUT, > OUTS, CLI, STI. > > > > +bool kvm_check_iopl(struct kvm_vcpu *vcpu) > +{ > + int iopl; > + if (!is_protmode(vcpu)) > + return false; > + if (kvm_get_rflags(vcpu)& X86_EFLAGS_VM) > + return true; > + iopl = (kvm_get_rflags(vcpu)& X86_EFLAGS_IOPL)>> IOPL_SHIFT; > + return kvm_x86_ops->get_cpl(vcpu)> iopl; > +} > Confusingly named - check doesn't imply what the return value means (and 'true' is surprising for a failure). Suggest kvm_bad_iopl() or similar. > + > +bool kvm_check_io_port_access_allowed(struct kvm_vcpu *vcpu, u16 port, u16 len) > +{ > Similarly, can drop check_ from the name. > + struct kvm_segment tr_seg; > + int r; > + u16 io_bitmap_ptr; > + u8 perm, bit_idx = port& 0x7; > + unsigned mask = (1<< len) - 1; > + > + kvm_get_segment(vcpu,&tr_seg, VCPU_SREG_TR); > + if (tr_seg.unusable) > + return false; > + if (tr_seg.limit< 103) > + return false; > + r = kvm_read_guest_virt_system(tr_seg.base + 102,&io_bitmap_ptr, 2, > + vcpu, NULL); > + if (r != X86EMUL_CONTINUE) > + return false; > + if (io_bitmap_ptr + port/8>= tr_seg.limit) > + return false; > Should this be '>'? limits are generally inclusive of the byte read (i.e. they aren't the size of the segment, but the offset of the last byte). > + r = kvm_read_guest_virt_system(tr_seg.base + io_bitmap_ptr + port/8, > + &perm, 1, vcpu, NULL); > + if (r != X86EMUL_CONTINUE) > + return false; > + if ((perm>> bit_idx)& mask) > + return false; > + return true; > +} > + > -- error compiling committee.c: too many arguments to function