[6/7] KVM: selftests: Expose the primary memslot number to tests
diff mbox series

Message ID 20200320205546.2396-7-sean.j.christopherson@intel.com
State New
Headers show
Series
  • KVM: Fix memslot use-after-free bug
Related show

Commit Message

Sean Christopherson March 20, 2020, 8:55 p.m. UTC
Add a define for the primary memslot number so that tests can manipulate
the memslot, e.g. to delete it.

Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
---
 tools/testing/selftests/kvm/include/kvm_util.h | 2 ++
 tools/testing/selftests/kvm/lib/kvm_util.c     | 4 ++--
 2 files changed, 4 insertions(+), 2 deletions(-)

Comments

Peter Xu March 23, 2020, 7:12 p.m. UTC | #1
On Fri, Mar 20, 2020 at 01:55:45PM -0700, Sean Christopherson wrote:
> Add a define for the primary memslot number so that tests can manipulate
> the memslot, e.g. to delete it.
> 
> Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
> ---
>  tools/testing/selftests/kvm/include/kvm_util.h | 2 ++
>  tools/testing/selftests/kvm/lib/kvm_util.c     | 4 ++--
>  2 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/testing/selftests/kvm/include/kvm_util.h b/tools/testing/selftests/kvm/include/kvm_util.h
> index 0f0e86e188c4..43b5feb546c6 100644
> --- a/tools/testing/selftests/kvm/include/kvm_util.h
> +++ b/tools/testing/selftests/kvm/include/kvm_util.h
> @@ -60,6 +60,8 @@ enum vm_mem_backing_src_type {
>  	VM_MEM_SRC_ANONYMOUS_HUGETLB,
>  };
>  
> +#define VM_PRIMARY_MEM_SLOT	0
> +
>  int kvm_check_cap(long cap);
>  int vm_enable_cap(struct kvm_vm *vm, struct kvm_enable_cap *cap);
>  
> diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c
> index f69fa84c9a4c..6a1af0455e44 100644
> --- a/tools/testing/selftests/kvm/lib/kvm_util.c
> +++ b/tools/testing/selftests/kvm/lib/kvm_util.c
> @@ -247,8 +247,8 @@ struct kvm_vm *_vm_create(enum vm_guest_mode mode, uint64_t phy_pages, int perm)
>  	/* Allocate and setup memory for guest. */
>  	vm->vpages_mapped = sparsebit_alloc();
>  	if (phy_pages != 0)
> -		vm_userspace_mem_region_add(vm, VM_MEM_SRC_ANONYMOUS,
> -					    0, 0, phy_pages, 0);
> +		vm_userspace_mem_region_add(vm, VM_MEM_SRC_ANONYMOUS, 0,
> +					    VM_PRIMARY_MEM_SLOT, phy_pages, 0);

IIUC VM_PRIMARY_MEM_SLOT should be used more than here... E.g., to all
the places that allocate page tables in virt_map() as the last param?
I didn't check other places.

Maybe it's simpler to drop this patch for now and use 0 directly as
before for now, after all in the last patch the comment is good enough
for me to understand slot 0 is the default slot.

Thanks,

>  
>  	return vm;
>  }
> -- 
> 2.24.1
>
Sean Christopherson March 23, 2020, 9:28 p.m. UTC | #2
On Mon, Mar 23, 2020 at 03:12:02PM -0400, Peter Xu wrote:
> On Fri, Mar 20, 2020 at 01:55:45PM -0700, Sean Christopherson wrote:
> > Add a define for the primary memslot number so that tests can manipulate
> > the memslot, e.g. to delete it.
> > 
> > Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
> > ---
> >  tools/testing/selftests/kvm/include/kvm_util.h | 2 ++
> >  tools/testing/selftests/kvm/lib/kvm_util.c     | 4 ++--
> >  2 files changed, 4 insertions(+), 2 deletions(-)
> > 
> > diff --git a/tools/testing/selftests/kvm/include/kvm_util.h b/tools/testing/selftests/kvm/include/kvm_util.h
> > index 0f0e86e188c4..43b5feb546c6 100644
> > --- a/tools/testing/selftests/kvm/include/kvm_util.h
> > +++ b/tools/testing/selftests/kvm/include/kvm_util.h
> > @@ -60,6 +60,8 @@ enum vm_mem_backing_src_type {
> >  	VM_MEM_SRC_ANONYMOUS_HUGETLB,
> >  };
> >  
> > +#define VM_PRIMARY_MEM_SLOT	0
> > +
> >  int kvm_check_cap(long cap);
> >  int vm_enable_cap(struct kvm_vm *vm, struct kvm_enable_cap *cap);
> >  
> > diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c
> > index f69fa84c9a4c..6a1af0455e44 100644
> > --- a/tools/testing/selftests/kvm/lib/kvm_util.c
> > +++ b/tools/testing/selftests/kvm/lib/kvm_util.c
> > @@ -247,8 +247,8 @@ struct kvm_vm *_vm_create(enum vm_guest_mode mode, uint64_t phy_pages, int perm)
> >  	/* Allocate and setup memory for guest. */
> >  	vm->vpages_mapped = sparsebit_alloc();
> >  	if (phy_pages != 0)
> > -		vm_userspace_mem_region_add(vm, VM_MEM_SRC_ANONYMOUS,
> > -					    0, 0, phy_pages, 0);
> > +		vm_userspace_mem_region_add(vm, VM_MEM_SRC_ANONYMOUS, 0,
> > +					    VM_PRIMARY_MEM_SLOT, phy_pages, 0);
> 
> IIUC VM_PRIMARY_MEM_SLOT should be used more than here... E.g., to all
> the places that allocate page tables in virt_map() as the last param?
> I didn't check other places.

Ouch, yeah, it bleeds into vm_vaddr_alloc() as well.
 
> Maybe it's simpler to drop this patch for now and use 0 directly as
> before for now, after all in the last patch the comment is good enough
> for me to understand slot 0 is the default slot.

Ya, I'll drop this and hardcode '0', it's a rather absurd amount of call
sites to change.

Patch
diff mbox series

diff --git a/tools/testing/selftests/kvm/include/kvm_util.h b/tools/testing/selftests/kvm/include/kvm_util.h
index 0f0e86e188c4..43b5feb546c6 100644
--- a/tools/testing/selftests/kvm/include/kvm_util.h
+++ b/tools/testing/selftests/kvm/include/kvm_util.h
@@ -60,6 +60,8 @@  enum vm_mem_backing_src_type {
 	VM_MEM_SRC_ANONYMOUS_HUGETLB,
 };
 
+#define VM_PRIMARY_MEM_SLOT	0
+
 int kvm_check_cap(long cap);
 int vm_enable_cap(struct kvm_vm *vm, struct kvm_enable_cap *cap);
 
diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c
index f69fa84c9a4c..6a1af0455e44 100644
--- a/tools/testing/selftests/kvm/lib/kvm_util.c
+++ b/tools/testing/selftests/kvm/lib/kvm_util.c
@@ -247,8 +247,8 @@  struct kvm_vm *_vm_create(enum vm_guest_mode mode, uint64_t phy_pages, int perm)
 	/* Allocate and setup memory for guest. */
 	vm->vpages_mapped = sparsebit_alloc();
 	if (phy_pages != 0)
-		vm_userspace_mem_region_add(vm, VM_MEM_SRC_ANONYMOUS,
-					    0, 0, phy_pages, 0);
+		vm_userspace_mem_region_add(vm, VM_MEM_SRC_ANONYMOUS, 0,
+					    VM_PRIMARY_MEM_SLOT, phy_pages, 0);
 
 	return vm;
 }