Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Getting ranges of valid guest physical memory address #23

Open
kylerky opened this issue Nov 27, 2019 · 8 comments
Open

Getting ranges of valid guest physical memory address #23

kylerky opened this issue Nov 27, 2019 · 8 comments

Comments

@kylerky
Copy link

kylerky commented Nov 27, 2019

Is there any way to know which guest physical memory addresses are valid for KVMI_READ_PHYSICAL and KVMI_WRITE_PHYSICAL?

As far as I can see, KVMI_GET_MAX_GFN gives the largest physical memory address , say max_addr (max_gfn << 12), used by the guest. But I found that I could get an error code -22 from KVM when I am reading from some addresses smaller than max_addr. I suspect that this is because those addresses are not mapped by the KVM.

Currently, KVMI_READ_PHYSICAL returns error code -22 when reading from invalid addresses and the socket will be closed by KVM, which is not convenient when I try to scan the whole memory of the guest.

Given that there is no way to get ranges of valid guest memory address, I think a better way is not to close the socket when reading from invalid addresses.

@mtarral
Copy link
Collaborator

mtarral commented Nov 27, 2019

ping @adlazar, @mdontu

@mdontu
Copy link

mdontu commented Nov 27, 2019

@kylerky You are correct and I think the current behaviour is the result of a bug. The API documentation indicates that the connection is closed on communication errors, however the calls themselves should be allowed to fail and some do indeed (the memory mapping ones come to mind).
I will revise the behaviour with @adlazar.

@adlazar
Copy link
Collaborator

adlazar commented Dec 2, 2019

I can't reproduce it unless the gpa/size pair is not valid. We use the following function to validate:

static bool invalid_page_access(u64 gpa, u64 size)
{
	u64 off = gpa & ~PAGE_MASK;

	return (size == 0 || size > PAGE_SIZE || off + size > PAGE_SIZE);
}

Should we return an error code (as a command reply) instead for this case too?

There is also a bug when you try to read a whole page, becauseKVMI_MSG_SIZE is too small. I'll try to create a pull request for this.

@kylerky
Copy link
Author

kylerky commented Dec 2, 2019

Should we return an error code (as a command reply) instead for this case too?

I think so.

There is also a bug when you try to read a whole page, becauseKVMI_MSG_SIZE is too small. I'll try to create a pull request for this.

That is what I was doing. I was reading the memory in 4KB chunks when the socket was closed. This does not happen with all the 4KB reads though, just some of them.

@adlazar
Copy link
Collaborator

adlazar commented Dec 2, 2019

#25 should fix the issue related to KVMI_MSG_SIZE

@adlazar
Copy link
Collaborator

adlazar commented Dec 2, 2019

Should we return an error code (as a command reply) instead for this case too?

I think so.

So, returning EINVAL when the gpa/pair is not valid and ENOENT when the page is not mapped sounds good ?

@kylerky
Copy link
Author

kylerky commented Dec 2, 2019

So, returning EINVAL when the gpa/pair is not valid and ENOENT when the page is not mapped sounds good ?

Good. It is helpful that we can distinguish between the two kinds of errors.

@adlazar
Copy link
Collaborator

adlazar commented Dec 2, 2019

I'll queue this change for the next version, because I don't think you need it right now. You still have to use arguments that pass the validation tests in order to see if the page is mapped or not. Right? #25 will allow you to read a whole page.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants