diff mbox

virt/kvm avoids oops by adding parameter checking

Message ID 20170822010909.103060-1-nixiaoming@huawei.com (mailing list archive)
State New, archived
Headers show

Commit Message

Xiaoming Ni Aug. 22, 2017, 1:09 a.m. UTC
The error parameter passed through the external interface causes the system oops.
So it is necessary to increase the parameter check for all EXPORT_SYMBOL_GPL

example:
 void kvm_get_kvm(struct kvm *kvm)
 {
        refcount_inc(&kvm->users_count); /*oops if kvm == NULL */
 }
 EXPORT_SYMBOL_GPL(kvm_get_kvm);

Signed-off-by: nixiaoming <nixiaoming@huawei.com>
---
 virt/kvm/eventfd.c  |  2 ++
 virt/kvm/kvm_main.c | 52 +++++++++++++++++++++++++++++++++++++++++++++++++---
 2 files changed, 51 insertions(+), 3 deletions(-)

Comments

Paolo Bonzini Aug. 22, 2017, 1:37 p.m. UTC | #1
On 22/08/2017 03:09, nixiaoming wrote:
> The error parameter passed through the external interface causes the system oops.
> So it is necessary to increase the parameter check for all EXPORT_SYMBOL_GPL
> 
> example:
>  void kvm_get_kvm(struct kvm *kvm)
>  {
>         refcount_inc(&kvm->users_count); /*oops if kvm == NULL */
>  }
>  EXPORT_SYMBOL_GPL(kvm_get_kvm);
> 
> Signed-off-by: nixiaoming <nixiaoming@huawei.com>

No.  It's simply a precondition that kvm != NULL in this case.

Paolo
kernel test robot Sept. 3, 2017, 3:36 a.m. UTC | #2
Hi nixiaoming,

[auto build test ERROR on v4.13-rc6]
[also build test ERROR on next-20170901]
[cannot apply to kvm/linux-next]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/nixiaoming/virt-kvm-avoids-oops-by-adding-parameter-checking/20170823-203000
config: powerpc-ppc64_defconfig (attached as .config)
compiler: powerpc64-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=powerpc 

All errors (new ones prefixed by >>):

   arch/powerpc/kvm/../../../virt/kvm/kvm_main.c: In function 'kvm_flush_remote_tlbs':
>> arch/powerpc/kvm/../../../virt/kvm/kvm_main.c:248:2: error: ISO C90 forbids mixed declarations and code [-Werror=declaration-after-statement]
     long dirty_count = smp_load_acquire(&kvm->tlbs_dirty);
     ^~~~
   arch/powerpc/kvm/../../../virt/kvm/kvm_main.c: In function 'kvm_write_guest_offset_cached':
   arch/powerpc/kvm/../../../virt/kvm/kvm_main.c:2025:2: error: ISO C90 forbids mixed declarations and code [-Werror=declaration-after-statement]
     gpa_t gpa = ghc->gpa + offset;
     ^~~~~
   cc1: all warnings being treated as errors

vim +248 arch/powerpc/kvm/../../../virt/kvm/kvm_main.c

d9e368d6 drivers/kvm/kvm_main.c Avi Kivity       2007-06-07  238  
a6d51016 virt/kvm/kvm_main.c    Mario Smarduch   2015-01-15  239  #ifndef CONFIG_HAVE_KVM_ARCH_TLB_FLUSH_ALL
49846896 virt/kvm/kvm_main.c    Rusty Russell    2008-12-08  240  void kvm_flush_remote_tlbs(struct kvm *kvm)
2e53d63a virt/kvm/kvm_main.c    Marcelo Tosatti  2008-02-20  241  {
6f1ad410 virt/kvm/kvm_main.c    nixiaoming       2017-08-22  242  	if (kvm == NULL)
6f1ad410 virt/kvm/kvm_main.c    nixiaoming       2017-08-22  243  		return;
4ae3cb3a virt/kvm/kvm_main.c    Lan Tianyu       2016-03-13  244  	/*
4ae3cb3a virt/kvm/kvm_main.c    Lan Tianyu       2016-03-13  245  	 * Read tlbs_dirty before setting KVM_REQ_TLB_FLUSH in
4ae3cb3a virt/kvm/kvm_main.c    Lan Tianyu       2016-03-13  246  	 * kvm_make_all_cpus_request.
4ae3cb3a virt/kvm/kvm_main.c    Lan Tianyu       2016-03-13  247  	 */
4ae3cb3a virt/kvm/kvm_main.c    Lan Tianyu       2016-03-13 @248  	long dirty_count = smp_load_acquire(&kvm->tlbs_dirty);
a086f6a1 virt/kvm/kvm_main.c    Xiao Guangrong   2014-04-17  249  
4ae3cb3a virt/kvm/kvm_main.c    Lan Tianyu       2016-03-13  250  	/*
4ae3cb3a virt/kvm/kvm_main.c    Lan Tianyu       2016-03-13  251  	 * We want to publish modifications to the page tables before reading
4ae3cb3a virt/kvm/kvm_main.c    Lan Tianyu       2016-03-13  252  	 * mode. Pairs with a memory barrier in arch-specific code.
4ae3cb3a virt/kvm/kvm_main.c    Lan Tianyu       2016-03-13  253  	 * - x86: smp_mb__after_srcu_read_unlock in vcpu_enter_guest
4ae3cb3a virt/kvm/kvm_main.c    Lan Tianyu       2016-03-13  254  	 * and smp_mb in walk_shadow_page_lockless_begin/end.
4ae3cb3a virt/kvm/kvm_main.c    Lan Tianyu       2016-03-13  255  	 * - powerpc: smp_mb in kvmppc_prepare_to_enter.
4ae3cb3a virt/kvm/kvm_main.c    Lan Tianyu       2016-03-13  256  	 *
4ae3cb3a virt/kvm/kvm_main.c    Lan Tianyu       2016-03-13  257  	 * There is already an smp_mb__after_atomic() before
4ae3cb3a virt/kvm/kvm_main.c    Lan Tianyu       2016-03-13  258  	 * kvm_make_all_cpus_request() reads vcpu->mode. We reuse that
4ae3cb3a virt/kvm/kvm_main.c    Lan Tianyu       2016-03-13  259  	 * barrier here.
4ae3cb3a virt/kvm/kvm_main.c    Lan Tianyu       2016-03-13  260  	 */
445b8236 virt/kvm/kvm_main.c    Tang Chen        2014-09-24  261  	if (kvm_make_all_cpus_request(kvm, KVM_REQ_TLB_FLUSH))
49846896 virt/kvm/kvm_main.c    Rusty Russell    2008-12-08  262  		++kvm->stat.remote_tlb_flush;
a086f6a1 virt/kvm/kvm_main.c    Xiao Guangrong   2014-04-17  263  	cmpxchg(&kvm->tlbs_dirty, dirty_count, 0);
2e53d63a virt/kvm/kvm_main.c    Marcelo Tosatti  2008-02-20  264  }
2ba9f0d8 virt/kvm/kvm_main.c    Aneesh Kumar K.V 2013-10-07  265  EXPORT_SYMBOL_GPL(kvm_flush_remote_tlbs);
a6d51016 virt/kvm/kvm_main.c    Mario Smarduch   2015-01-15  266  #endif
2e53d63a virt/kvm/kvm_main.c    Marcelo Tosatti  2008-02-20  267  

:::::: The code at line 248 was first introduced by commit
:::::: 4ae3cb3a2551b41f22284f713e7d5e2b61a85c1d KVM: Replace smp_mb() with smp_load_acquire() in the kvm_flush_remote_tlbs()

:::::: TO: Lan Tianyu <tianyu.lan@intel.com>
:::::: CC: Paolo Bonzini <pbonzini@redhat.com>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
kernel test robot Sept. 3, 2017, 4 a.m. UTC | #3
Hi nixiaoming,

[auto build test ERROR on v4.13-rc6]
[also build test ERROR on next-20170901]
[cannot apply to kvm/linux-next]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/nixiaoming/virt-kvm-avoids-oops-by-adding-parameter-checking/20170823-203000
config: mips-malta_kvm_defconfig (attached as .config)
compiler: mipsel-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=mips 

All errors (new ones prefixed by >>):

   arch/mips/kvm/../../../virt/kvm/kvm_main.c: In function 'kvm_flush_remote_tlbs':
>> arch/mips/kvm/../../../virt/kvm/kvm_main.c:248:2: error: ISO C90 forbids mixed declarations and code [-Werror=declaration-after-statement]
     long dirty_count = smp_load_acquire(&kvm->tlbs_dirty);
     ^~~~
   arch/mips/kvm/../../../virt/kvm/kvm_main.c: In function 'kvm_write_guest_offset_cached':
   arch/mips/kvm/../../../virt/kvm/kvm_main.c:2025:2: error: ISO C90 forbids mixed declarations and code [-Werror=declaration-after-statement]
     gpa_t gpa = ghc->gpa + offset;
     ^~~~~
   cc1: all warnings being treated as errors

vim +248 arch/mips/kvm/../../../virt/kvm/kvm_main.c

d9e368d6 drivers/kvm/kvm_main.c Avi Kivity       2007-06-07  238  
a6d51016 virt/kvm/kvm_main.c    Mario Smarduch   2015-01-15  239  #ifndef CONFIG_HAVE_KVM_ARCH_TLB_FLUSH_ALL
49846896 virt/kvm/kvm_main.c    Rusty Russell    2008-12-08  240  void kvm_flush_remote_tlbs(struct kvm *kvm)
2e53d63a virt/kvm/kvm_main.c    Marcelo Tosatti  2008-02-20  241  {
6f1ad410 virt/kvm/kvm_main.c    nixiaoming       2017-08-22  242  	if (kvm == NULL)
6f1ad410 virt/kvm/kvm_main.c    nixiaoming       2017-08-22  243  		return;
4ae3cb3a virt/kvm/kvm_main.c    Lan Tianyu       2016-03-13  244  	/*
4ae3cb3a virt/kvm/kvm_main.c    Lan Tianyu       2016-03-13  245  	 * Read tlbs_dirty before setting KVM_REQ_TLB_FLUSH in
4ae3cb3a virt/kvm/kvm_main.c    Lan Tianyu       2016-03-13  246  	 * kvm_make_all_cpus_request.
4ae3cb3a virt/kvm/kvm_main.c    Lan Tianyu       2016-03-13  247  	 */
4ae3cb3a virt/kvm/kvm_main.c    Lan Tianyu       2016-03-13 @248  	long dirty_count = smp_load_acquire(&kvm->tlbs_dirty);
a086f6a1 virt/kvm/kvm_main.c    Xiao Guangrong   2014-04-17  249  
4ae3cb3a virt/kvm/kvm_main.c    Lan Tianyu       2016-03-13  250  	/*
4ae3cb3a virt/kvm/kvm_main.c    Lan Tianyu       2016-03-13  251  	 * We want to publish modifications to the page tables before reading
4ae3cb3a virt/kvm/kvm_main.c    Lan Tianyu       2016-03-13  252  	 * mode. Pairs with a memory barrier in arch-specific code.
4ae3cb3a virt/kvm/kvm_main.c    Lan Tianyu       2016-03-13  253  	 * - x86: smp_mb__after_srcu_read_unlock in vcpu_enter_guest
4ae3cb3a virt/kvm/kvm_main.c    Lan Tianyu       2016-03-13  254  	 * and smp_mb in walk_shadow_page_lockless_begin/end.
4ae3cb3a virt/kvm/kvm_main.c    Lan Tianyu       2016-03-13  255  	 * - powerpc: smp_mb in kvmppc_prepare_to_enter.
4ae3cb3a virt/kvm/kvm_main.c    Lan Tianyu       2016-03-13  256  	 *
4ae3cb3a virt/kvm/kvm_main.c    Lan Tianyu       2016-03-13  257  	 * There is already an smp_mb__after_atomic() before
4ae3cb3a virt/kvm/kvm_main.c    Lan Tianyu       2016-03-13  258  	 * kvm_make_all_cpus_request() reads vcpu->mode. We reuse that
4ae3cb3a virt/kvm/kvm_main.c    Lan Tianyu       2016-03-13  259  	 * barrier here.
4ae3cb3a virt/kvm/kvm_main.c    Lan Tianyu       2016-03-13  260  	 */
445b8236 virt/kvm/kvm_main.c    Tang Chen        2014-09-24  261  	if (kvm_make_all_cpus_request(kvm, KVM_REQ_TLB_FLUSH))
49846896 virt/kvm/kvm_main.c    Rusty Russell    2008-12-08  262  		++kvm->stat.remote_tlb_flush;
a086f6a1 virt/kvm/kvm_main.c    Xiao Guangrong   2014-04-17  263  	cmpxchg(&kvm->tlbs_dirty, dirty_count, 0);
2e53d63a virt/kvm/kvm_main.c    Marcelo Tosatti  2008-02-20  264  }
2ba9f0d8 virt/kvm/kvm_main.c    Aneesh Kumar K.V 2013-10-07  265  EXPORT_SYMBOL_GPL(kvm_flush_remote_tlbs);
a6d51016 virt/kvm/kvm_main.c    Mario Smarduch   2015-01-15  266  #endif
2e53d63a virt/kvm/kvm_main.c    Marcelo Tosatti  2008-02-20  267  

:::::: The code at line 248 was first introduced by commit
:::::: 4ae3cb3a2551b41f22284f713e7d5e2b61a85c1d KVM: Replace smp_mb() with smp_load_acquire() in the kvm_flush_remote_tlbs()

:::::: TO: Lan Tianyu <tianyu.lan@intel.com>
:::::: CC: Paolo Bonzini <pbonzini@redhat.com>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
diff mbox

Patch

diff --git a/virt/kvm/eventfd.c b/virt/kvm/eventfd.c
index f2ac53a..250200b 100644
--- a/virt/kvm/eventfd.c
+++ b/virt/kvm/eventfd.c
@@ -444,6 +444,8 @@  bool kvm_irq_has_notifier(struct kvm *kvm, unsigned irqchip, unsigned pin)
 {
 	struct kvm_irq_ack_notifier *kian;
 	int gsi, idx;
+	if (kvm == NULL)
+		return false;
 
 	idx = srcu_read_lock(&kvm->irq_srcu);
 	gsi = kvm_irq_map_chip_pin(kvm, irqchip, pin);
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 15252d7..3e25de0 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -150,6 +150,8 @@  bool kvm_is_reserved_pfn(kvm_pfn_t pfn)
 int vcpu_load(struct kvm_vcpu *vcpu)
 {
 	int cpu;
+	if (vcpu == NULL)
+		return -EINVAL;
 
 	if (mutex_lock_killable(&vcpu->mutex))
 		return -EINTR;
@@ -163,6 +165,8 @@  EXPORT_SYMBOL_GPL(vcpu_load);
 
 void vcpu_put(struct kvm_vcpu *vcpu)
 {
+	if (vcpu == NULL)
+		return;
 	preempt_disable();
 	kvm_arch_vcpu_put(vcpu);
 	preempt_notifier_unregister(&vcpu->preempt_notifier);
@@ -235,6 +239,8 @@  bool kvm_make_all_cpus_request(struct kvm *kvm, unsigned int req)
 #ifndef CONFIG_HAVE_KVM_ARCH_TLB_FLUSH_ALL
 void kvm_flush_remote_tlbs(struct kvm *kvm)
 {
+	if (kvm == NULL)
+		return;
 	/*
 	 * Read tlbs_dirty before setting KVM_REQ_TLB_FLUSH in
 	 * kvm_make_all_cpus_request.
@@ -269,6 +275,9 @@  int kvm_vcpu_init(struct kvm_vcpu *vcpu, struct kvm *kvm, unsigned id)
 	struct page *page;
 	int r;
 
+	if (vcpu == NULL)
+		return -EINVAL;
+
 	mutex_init(&vcpu->mutex);
 	vcpu->cpu = -1;
 	vcpu->kvm = kvm;
@@ -305,6 +314,8 @@  EXPORT_SYMBOL_GPL(kvm_vcpu_init);
 
 void kvm_vcpu_uninit(struct kvm_vcpu *vcpu)
 {
+	if (vcpu == NULL)
+		return;
 	/*
 	 * no need for rcu_read_lock as VCPU_RUN is the only place that
 	 * will change the vcpu->pid pointer and on uninit all file
@@ -779,12 +790,16 @@  static void kvm_destroy_vm(struct kvm *kvm)
 
 void kvm_get_kvm(struct kvm *kvm)
 {
+	if (kvm == NULL)
+		return;
 	refcount_inc(&kvm->users_count);
 }
 EXPORT_SYMBOL_GPL(kvm_get_kvm);
 
 void kvm_put_kvm(struct kvm *kvm)
 {
+	if (kvm == NULL)
+		return;
 	if (refcount_dec_and_test(&kvm->users_count))
 		kvm_destroy_vm(kvm);
 }
@@ -938,6 +953,9 @@  int __kvm_set_memory_region(struct kvm *kvm,
 	int as_id, id;
 	enum kvm_mr_change change;
 
+	if (mem == NULL)
+		return -EINVAL;
+
 	r = check_memory_region_flags(mem);
 	if (r)
 		goto out;
@@ -1121,6 +1139,9 @@  int kvm_get_dirty_log(struct kvm *kvm,
 	unsigned long n;
 	unsigned long any = 0;
 
+	if (log == NULL || is_dirty == NULL)
+		return -EINVAL;
+
 	as_id = log->slot >> 16;
 	id = (u16)log->slot;
 	if (as_id >= KVM_ADDRESS_SPACE_NUM || id >= KVM_USER_MEM_SLOTS)
@@ -1178,6 +1199,9 @@  int kvm_get_dirty_log_protect(struct kvm *kvm,
 	unsigned long *dirty_bitmap;
 	unsigned long *dirty_bitmap_buffer;
 
+	if (log == NULL || is_dirty == NULL)
+		return -EINVAL;
+
 	as_id = log->slot >> 16;
 	id = (u16)log->slot;
 	if (as_id >= KVM_ADDRESS_SPACE_NUM || id >= KVM_USER_MEM_SLOTS)
@@ -1996,6 +2020,8 @@  int kvm_write_guest_offset_cached(struct kvm *kvm, struct gfn_to_hva_cache *ghc,
 {
 	struct kvm_memslots *slots = kvm_memslots(kvm);
 	int r;
+	if (ghc == NULL)
+		return -EINVAL;
 	gpa_t gpa = ghc->gpa + offset;
 
 	BUG_ON(len + offset > ghc->len);
@@ -2225,6 +2251,8 @@  EXPORT_SYMBOL_GPL(kvm_vcpu_block);
 bool kvm_vcpu_wake_up(struct kvm_vcpu *vcpu)
 {
 	struct swait_queue_head *wqp;
+	if (vcpu == NULL)
+		return false;
 
 	wqp = kvm_arch_vcpu_wq(vcpu);
 	if (swait_active(wqp)) {
@@ -2244,7 +2272,11 @@  EXPORT_SYMBOL_GPL(kvm_vcpu_wake_up);
 void kvm_vcpu_kick(struct kvm_vcpu *vcpu)
 {
 	int me;
-	int cpu = vcpu->cpu;
+	int cpu;
+
+	if (vcpu == NULL)
+		return;
+	cpu = vcpu->cpu;
 
 	if (kvm_vcpu_wake_up(vcpu))
 		return;
@@ -2264,6 +2296,9 @@  int kvm_vcpu_yield_to(struct kvm_vcpu *target)
 	struct task_struct *task = NULL;
 	int ret = 0;
 
+	if (target == NULL)
+		return -EINVAL;
+
 	rcu_read_lock();
 	pid = rcu_dereference(target->pid);
 	if (pid)
@@ -2319,13 +2354,19 @@  static bool kvm_vcpu_eligible_for_directed_yield(struct kvm_vcpu *vcpu)
 
 void kvm_vcpu_on_spin(struct kvm_vcpu *me)
 {
-	struct kvm *kvm = me->kvm;
+	struct kvm *kvm;
 	struct kvm_vcpu *vcpu;
-	int last_boosted_vcpu = me->kvm->last_boosted_vcpu;
+	int last_boosted_vcpu;
 	int yielded = 0;
 	int try = 3;
 	int pass;
 	int i;
+	if (me == NULL)
+		return;
+	kvm = me->kvm;
+	if (kvm == NULL)
+		return;
+	last_boosted_vcpu = me->kvm->last_boosted_vcpu;
 
 	kvm_vcpu_set_in_spin_loop(me, true);
 	/*
@@ -3542,6 +3583,8 @@  static int __kvm_io_bus_read(struct kvm_vcpu *vcpu, struct kvm_io_bus *bus,
 			     struct kvm_io_range *range, void *val)
 {
 	int idx;
+	if (range == NULL)
+		return -EINVAL;
 
 	idx = kvm_io_bus_get_first_dev(bus, range->addr, range->len);
 	if (idx < 0)
@@ -3653,6 +3696,9 @@  struct kvm_io_device *kvm_io_bus_get_dev(struct kvm *kvm, enum kvm_bus bus_idx,
 	int dev_idx, srcu_idx;
 	struct kvm_io_device *iodev = NULL;
 
+	if (kvm == NULL)
+		return NULL;
+
 	srcu_idx = srcu_read_lock(&kvm->srcu);
 
 	bus = srcu_dereference(kvm->buses[bus_idx], &kvm->srcu);