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

Re: [Xen-devel] [PATCH v4 04/16] xen: Add is_vmware_port_enabled

On 09/19/2014 09:24 AM, Slutz, Donald Christopher wrote:
On 09/18/14 18:53, Boris Ostrovsky wrote:
On 09/17/2014 02:23 PM, Slutz, Donald Christopher wrote:

I just spent some time checking and found out that even with the cpu

(XEN)   - Next-RIP Saved on #VMEXIT

svm_nextrip_insn_length(v) is 0.
As I see it, this could happen for one of three reasons:
* !cpu_has_svm_nrips which can't be the case since (1) family 21
supports it and (2) you actually see in the log that it does have it
* next RIP is before current RIP which I think can't be the case
neither because we are not looking at branch instruction or something
like that.
* nextrip == rip. Which I don't see how it can be true.

Can you check why svm_nextrip_insn_length(v) is 0?

What I have found out is that vmcb->nextrip is 0 in my testing. So
next RIP is "before current RIP" by a very large amount.

Ah, I just realized --- you are in VMEXIT because of #GP intercept, not because of IOIO intercept. And during #GP NRIP does not get updated (it is set to zero).

Which means that you will always be doing decoding when you call __get_instruction_length_from_list(), regardless of NRIP support.

But regardless of that, how do you expect your code to work on CPUs
that don't support NRIP? On those processors you *will* be decoding
the instruction twice.

The code "works" because the only info passed between the 2 decodes is
the instruction length.  This is used to limit the amount of the 2nd read.

And because svm_nextrip_insn_length(v) is 0, all the testing I have done
on the AMD server has been doing the decode of the instruction twice.

If another CPU is changing the instructions as I read them (which is the
security issue as I understand it), all I see happing is that "wrong"
or size of request could happen, or it is a vmport request or not. All of
which either report a #GP or do the vmport action.  Since you can do the
vmport action without changing the instruction bytes, I do not see how
the double decode opens any security issue.

I am not trying to say this is good.  And as I replied to Jan, I am
looking into
a way to only do the single decode.  The simplest of these is to just not
use __get_instruction_length_from_list() and just state that on AMD the
instruction length is 2.  This is safe because I am only using this to
much many bytes on the page to read.  The 2nd page read read depends
on finding a 0x66 prefix byte.

Can you make this statement (about instruction length being 2) for both AMD and Intel and then possibly move some code into common HVM code?

Since we are intercepting #GP on both architectures only for VMware case (right?) it seems that you can just say -- "let's look at the next 2 bytes and confirm that they are IN/OUT (with appropriate arguments)".


I am just noticing that this also has the
effect of not injecting a #PF if the instruction is no longer readable
(a side
effect of using fetch() which uses hvm_fetch_from_guest_virt()).

Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.