lkml.org 
[lkml]   [2008]   [Nov]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] always assign userspace_addr
Glauber Costa wrote:
> Currently, kvm only sets new.userspace_addr in slots
> that were just allocated. This is not the intended behaviour,
> and actually breaks when we try to use the slots to implement
> aliases, for example.
>
> Cirrus VGA aliases maps and address to a userspace address, and
> then keep mapping this same address to different locations
> until the whole screen is filled.
>
> The solution is to assign new.userspace_addr no matter what,
> so we can be sure that whenever the guest changes this field,
> it sees the change being reflected in the code.
>
> Signed-off-by: Glauber Costa <glommer@redhat.com>
>

I think this is masking a much bigger problem.


> ---
> virt/kvm/kvm_main.c | 18 +++++++++---------
> 1 files changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index a87f45e..fc3abf0 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -762,15 +762,6 @@ int __kvm_set_memory_region(struct kvm *kvm,
> memset(new.rmap, 0, npages * sizeof(*new.rmap));
>
> new.user_alloc = user_alloc;
> - /*
> - * hva_to_rmmap() serialzies with the mmu_lock and to be
> - * safe it has to ignore memslots with !user_alloc &&
> - * !userspace_addr.
> - */
> - if (user_alloc)
> - new.userspace_addr = mem->userspace_addr;
> - else
> - new.userspace_addr = 0;
>

This is guarded in:

> if (npages && !new.rmap) {

In this case, npages > 0 but !new.rmap is already allocated. But this
is a new slot? The problem is that when we delete the slot, the rmap
never gets freed. This means that if we delete a slot, then create a
new slot which happens to be a different size, we use the old rmap and
potentially overrun that buffer.

So I think we need a fix that properly frees the rmap when the slot is
destroyed.

Regards,

Anthony Liguori



\
 
 \ /
  Last update: 2008-11-19 16:57    [W:0.065 / U:4.344 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site