Re: [PATCH] KVM: Veirfy memory slot only for readability

[Takuya Yoshikawa <yoshikawa.takuya@oss.ntt.co.jp>]

(2011/12/02 14:46), Sasha Levin wrote:
> On Fri, 2011-12-02 at 12:16 +0900, Takuya Yoshikawa wrote:
>> (2011/12/02 10:15), Takuya Yoshikawa wrote:
>>> (2011/12/02 4:42), Sasha Levin wrote:
>>>> It's enough for memory slot to be readable, as the comment above the check
>>>> states.
>>>>
>>>> A user should be able to create read-only memory slot.
>>
>> Note: at that time, it looked to me that the API did not allow me to know
>> which type was being registered.
>
> Which code exactly assumes that all memory slots are writable?

I agree with you, but as I wrote, my patch only used the check for
read with __xxx_user() because I had not thought changing the code
for write, not hot compared to read, would not worth it.

But access_ok() does the same for the two cases, so I did not mind
changing VERIFY_READ to VERIFY_WRITE: you should not have any problem
to do what you want, no?

I am not mentioning whether it is good as a programming style, here.

>
>> Do you want to create read only memory slots for kvm tool?
>
> What KVM tool currently does is copy the kernel into guest memory and
> run it from there. An idea raised recently was instead of copying it we
> should mmap it into the memory to reduce footprint.

Interesting.

>
> This is why I'm looking into adding a read only memory slot. The KVM
> code doesn't have to know it's read only.
>

What I wanted to say was that, it might be cleaner if I could put the
exactly needed flag only.

And if you want to change the VERIFY_WRITE to VERIFY_READ, please check
the following commit as well:

	commit ccddff20b9368cce9b774f6a22440a7c45367784
	Author: Xiao Guangrong <xiaoguangrong@cn.fujitsu.com>

	KVM: use __copy_to_user/__clear_user to write guest page

Ah, and the code is not x86 specific, so you may better check other
architectures as well.

Anyway, adding some more descriptions based on the history will be nice.

	Takuya