Re: [PATCH 2/6] KVM: x86: Wrong error code on limit violation during emulation

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> On Oct 27, 2014, at 16:37, Paolo Bonzini <pbonzini@xxxxxxxxxx> wrote:
> 
> 
> 
> On 09/30/2014 07:49 PM, Nadav Amit wrote:
>> GP and SS exceptions deliver as error-code the segment selector if the
>> exception occurred when the segment is loaded.  However, if the exception
>> occurs during the memory access itself, due to limit violations, the error-code
>> should be zero.  Fix it.
>> 
>> Signed-off-by: Nadav Amit <namit@xxxxxxxxxxxxxxxxx>
>> ---
>> arch/x86/kvm/emulate.c | 20 +++++++++++---------
>> 1 file changed, 11 insertions(+), 9 deletions(-)
>> 
>> diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
>> index a46207a..13a1c76 100644
>> --- a/arch/x86/kvm/emulate.c
>> +++ b/arch/x86/kvm/emulate.c
>> @@ -621,7 +621,7 @@ static int __linearize(struct x86_emulate_ctxt *ctxt,
>> 	bool usable;
>> 	ulong la;
>> 	u32 lim;
>> -	u16 sel;
>> +	u16 sel, error_code = 0;
>> 	unsigned cpl;
>> 
>> 	la = seg_base(ctxt, addr.seg) + addr.ea;
>> @@ -634,14 +634,14 @@ static int __linearize(struct x86_emulate_ctxt *ctxt,
>> 		usable = ctxt->ops->get_segment(ctxt, &sel, &desc, NULL,
>> 						addr.seg);
>> 		if (!usable)
>> -			goto bad;
>> +			goto bad_sel;
> 
> This can only happen because of a NULL selector, which means the error
> code is zero anyway.
> 
>> 		/* code segment in protected mode or read-only data segment */
>> 		if ((((ctxt->mode != X86EMUL_MODE_REAL) && (desc.type & 8))
>> 					|| !(desc.type & 2)) && write)
>> -			goto bad;
>> +			goto bad_sel;
> 
> This is not "detected while loading a segment descriptor", so the error
> code should be zero.
> 
>> 		/* unreadable code segment */
>> 		if (!fetch && (desc.type & 8) && !(desc.type & 2))
>> -			goto bad;
>> +			goto bad_sel;
> 
> Same here.
> 
>> 		lim = desc_limit_scaled(&desc);
>> 		if ((ctxt->mode == X86EMUL_MODE_REAL) && !fetch &&
>> 		    (ctxt->d & NoBigReal)) {
>> @@ -664,15 +664,15 @@ static int __linearize(struct x86_emulate_ctxt *ctxt,
>> 		if (!(desc.type & 8)) {
>> 			/* data segment */
>> 			if (cpl > desc.dpl)
>> -				goto bad;
>> +				goto bad_sel;
>> 		} else if ((desc.type & 8) && !(desc.type & 4)) {
>> 			/* nonconforming code segment */
>> 			if (cpl != desc.dpl)
>> -				goto bad;
>> +				goto bad_sel;
>> 		} else if ((desc.type & 8) && (desc.type & 4)) {
>> 			/* conforming code segment */
>> 			if (cpl < desc.dpl)
>> -				goto bad;
>> +				goto bad_sel;
> 
> These three should be deleted, as you pointed out in patch 5.
> 
> So I've dropped this patch, and posted a simpler alternative that just
> uses error code 0 in __linearize.  Can you look at it?

It looks fine. I noticed these mistakes also...
Please wait with the other emulator fixes I posted. I run some further tests, since I am annoyed of making mistakes and redoing patches.

Regards,
Nadav 

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux