All of lore.kernel.org
 help / color / mirror / Atom feed
* FAILED: patch "[PATCH] KVM: kvm_io_bus_unregister_dev() should never fail" failed to apply to 4.10-stable tree
@ 2017-03-30  8:00 gregkh
  2017-04-03  8:23 ` David Hildenbrand
  0 siblings, 1 reply; 3+ messages in thread
From: gregkh @ 2017-03-30  8:00 UTC (permalink / raw)
  To: david, cornelia.huck, dvyukov, pbonzini; +Cc: stable


The patch below does not apply to the 4.10-stable tree.
If someone wants it applied there, or to any other stable or longterm
tree, then please email the backport, including the original git commit
id to <stable@vger.kernel.org>.

thanks,

greg k-h

------------------ original commit in Linus's tree ------------------

>From 90db10434b163e46da413d34db8d0e77404cc645 Mon Sep 17 00:00:00 2001
From: David Hildenbrand <david@redhat.com>
Date: Thu, 23 Mar 2017 18:24:19 +0100
Subject: [PATCH] KVM: kvm_io_bus_unregister_dev() should never fail

No caller currently checks the return value of
kvm_io_bus_unregister_dev(). This is evil, as all callers silently go on
freeing their device. A stale reference will remain in the io_bus,
getting at least used again, when the iobus gets teared down on
kvm_destroy_vm() - leading to use after free errors.

There is nothing the callers could do, except retrying over and over
again.

So let's simply remove the bus altogether, print an error and make
sure no one can access this broken bus again (returning -ENOMEM on any
attempt to access it).

Fixes: e93f8a0f821e ("KVM: convert io_bus to SRCU")
Cc: stable@vger.kernel.org # 3.4+
Reported-by: Dmitry Vyukov <dvyukov@google.com>
Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>
Signed-off-by: David Hildenbrand <david@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 2c14ad9809da..d0250744507a 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -162,8 +162,8 @@ int kvm_io_bus_read(struct kvm_vcpu *vcpu, enum kvm_bus bus_idx, gpa_t addr,
 		    int len, void *val);
 int kvm_io_bus_register_dev(struct kvm *kvm, enum kvm_bus bus_idx, gpa_t addr,
 			    int len, struct kvm_io_device *dev);
-int kvm_io_bus_unregister_dev(struct kvm *kvm, enum kvm_bus bus_idx,
-			      struct kvm_io_device *dev);
+void kvm_io_bus_unregister_dev(struct kvm *kvm, enum kvm_bus bus_idx,
+			       struct kvm_io_device *dev);
 struct kvm_io_device *kvm_io_bus_get_dev(struct kvm *kvm, enum kvm_bus bus_idx,
 					 gpa_t addr);
 
diff --git a/virt/kvm/eventfd.c b/virt/kvm/eventfd.c
index a29786dd9522..4d28a9ddbee0 100644
--- a/virt/kvm/eventfd.c
+++ b/virt/kvm/eventfd.c
@@ -870,7 +870,8 @@ kvm_deassign_ioeventfd_idx(struct kvm *kvm, enum kvm_bus bus_idx,
 			continue;
 
 		kvm_io_bus_unregister_dev(kvm, bus_idx, &p->dev);
-		kvm->buses[bus_idx]->ioeventfd_count--;
+		if (kvm->buses[bus_idx])
+			kvm->buses[bus_idx]->ioeventfd_count--;
 		ioeventfd_release(p);
 		ret = 0;
 		break;
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 7445566fadc1..ef1aa7f1ed7a 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -728,7 +728,8 @@ static void kvm_destroy_vm(struct kvm *kvm)
 	spin_unlock(&kvm_lock);
 	kvm_free_irq_routing(kvm);
 	for (i = 0; i < KVM_NR_BUSES; i++) {
-		kvm_io_bus_destroy(kvm->buses[i]);
+		if (kvm->buses[i])
+			kvm_io_bus_destroy(kvm->buses[i]);
 		kvm->buses[i] = NULL;
 	}
 	kvm_coalesced_mmio_free(kvm);
@@ -3476,6 +3477,8 @@ int kvm_io_bus_write(struct kvm_vcpu *vcpu, enum kvm_bus bus_idx, gpa_t addr,
 	};
 
 	bus = srcu_dereference(vcpu->kvm->buses[bus_idx], &vcpu->kvm->srcu);
+	if (!bus)
+		return -ENOMEM;
 	r = __kvm_io_bus_write(vcpu, bus, &range, val);
 	return r < 0 ? r : 0;
 }
@@ -3493,6 +3496,8 @@ int kvm_io_bus_write_cookie(struct kvm_vcpu *vcpu, enum kvm_bus bus_idx,
 	};
 
 	bus = srcu_dereference(vcpu->kvm->buses[bus_idx], &vcpu->kvm->srcu);
+	if (!bus)
+		return -ENOMEM;
 
 	/* First try the device referenced by cookie. */
 	if ((cookie >= 0) && (cookie < bus->dev_count) &&
@@ -3543,6 +3548,8 @@ int kvm_io_bus_read(struct kvm_vcpu *vcpu, enum kvm_bus bus_idx, gpa_t addr,
 	};
 
 	bus = srcu_dereference(vcpu->kvm->buses[bus_idx], &vcpu->kvm->srcu);
+	if (!bus)
+		return -ENOMEM;
 	r = __kvm_io_bus_read(vcpu, bus, &range, val);
 	return r < 0 ? r : 0;
 }
@@ -3555,6 +3562,9 @@ int kvm_io_bus_register_dev(struct kvm *kvm, enum kvm_bus bus_idx, gpa_t addr,
 	struct kvm_io_bus *new_bus, *bus;
 
 	bus = kvm->buses[bus_idx];
+	if (!bus)
+		return -ENOMEM;
+
 	/* exclude ioeventfd which is limited by maximum fd */
 	if (bus->dev_count - bus->ioeventfd_count > NR_IOBUS_DEVS - 1)
 		return -ENOSPC;
@@ -3574,45 +3584,41 @@ int kvm_io_bus_register_dev(struct kvm *kvm, enum kvm_bus bus_idx, gpa_t addr,
 }
 
 /* Caller must hold slots_lock. */
-int kvm_io_bus_unregister_dev(struct kvm *kvm, enum kvm_bus bus_idx,
-			      struct kvm_io_device *dev)
+void kvm_io_bus_unregister_dev(struct kvm *kvm, enum kvm_bus bus_idx,
+			       struct kvm_io_device *dev)
 {
-	int i, r;
+	int i;
 	struct kvm_io_bus *new_bus, *bus;
 
 	bus = kvm->buses[bus_idx];
-
-	/*
-	 * It's possible the bus being released before hand. If so,
-	 * we're done here.
-	 */
 	if (!bus)
-		return 0;
+		return;
 
-	r = -ENOENT;
 	for (i = 0; i < bus->dev_count; i++)
 		if (bus->range[i].dev == dev) {
-			r = 0;
 			break;
 		}
 
-	if (r)
-		return r;
+	if (i == bus->dev_count)
+		return;
 
 	new_bus = kmalloc(sizeof(*bus) + ((bus->dev_count - 1) *
 			  sizeof(struct kvm_io_range)), GFP_KERNEL);
-	if (!new_bus)
-		return -ENOMEM;
+	if (!new_bus)  {
+		pr_err("kvm: failed to shrink bus, removing it completely\n");
+		goto broken;
+	}
 
 	memcpy(new_bus, bus, sizeof(*bus) + i * sizeof(struct kvm_io_range));
 	new_bus->dev_count--;
 	memcpy(new_bus->range + i, bus->range + i + 1,
 	       (new_bus->dev_count - i) * sizeof(struct kvm_io_range));
 
+broken:
 	rcu_assign_pointer(kvm->buses[bus_idx], new_bus);
 	synchronize_srcu_expedited(&kvm->srcu);
 	kfree(bus);
-	return r;
+	return;
 }
 
 struct kvm_io_device *kvm_io_bus_get_dev(struct kvm *kvm, enum kvm_bus bus_idx,
@@ -3625,6 +3631,8 @@ struct kvm_io_device *kvm_io_bus_get_dev(struct kvm *kvm, enum kvm_bus bus_idx,
 	srcu_idx = srcu_read_lock(&kvm->srcu);
 
 	bus = srcu_dereference(kvm->buses[bus_idx], &kvm->srcu);
+	if (!bus)
+		goto out_unlock;
 
 	dev_idx = kvm_io_bus_get_first_dev(bus, addr, 1);
 	if (dev_idx < 0)

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: FAILED: patch "[PATCH] KVM: kvm_io_bus_unregister_dev() should never fail" failed to apply to 4.10-stable tree
  2017-03-30  8:00 FAILED: patch "[PATCH] KVM: kvm_io_bus_unregister_dev() should never fail" failed to apply to 4.10-stable tree gregkh
@ 2017-04-03  8:23 ` David Hildenbrand
  2017-04-03 10:35   ` Paolo Bonzini
  0 siblings, 1 reply; 3+ messages in thread
From: David Hildenbrand @ 2017-04-03  8:23 UTC (permalink / raw)
  To: gregkh; +Cc: cornelia.huck, dvyukov, pbonzini, stable

On 30.03.2017 10:00, gregkh@linuxfoundation.org wrote:
> 
> The patch below does not apply to the 4.10-stable tree.
> If someone wants it applied there, or to any other stable or longterm
> tree, then please email the backport, including the original git commit
> id to <stable@vger.kernel.org>.
> 

Hi Greg,

this commit can be cherry-picked cleanly after cherry picking
df630b8c1e85 ("KVM: x86: clear bus pointer when destroyed"). Can you
also apply that commit or should I send a backport of the stable patch,
including the necessary check from the prerequisite patch?

Thanks!

> thanks,
> 
> greg k-h
> 
> ------------------ original commit in Linus's tree ------------------
> 
> From 90db10434b163e46da413d34db8d0e77404cc645 Mon Sep 17 00:00:00 2001
> From: David Hildenbrand <david@redhat.com>
> Date: Thu, 23 Mar 2017 18:24:19 +0100
> Subject: [PATCH] KVM: kvm_io_bus_unregister_dev() should never fail
> 
> No caller currently checks the return value of
> kvm_io_bus_unregister_dev(). This is evil, as all callers silently go on
> freeing their device. A stale reference will remain in the io_bus,
> getting at least used again, when the iobus gets teared down on
> kvm_destroy_vm() - leading to use after free errors.
> 
> There is nothing the callers could do, except retrying over and over
> again.
> 
> So let's simply remove the bus altogether, print an error and make
> sure no one can access this broken bus again (returning -ENOMEM on any
> attempt to access it).
> 
> Fixes: e93f8a0f821e ("KVM: convert io_bus to SRCU")
> Cc: stable@vger.kernel.org # 3.4+
> Reported-by: Dmitry Vyukov <dvyukov@google.com>
> Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>
> Signed-off-by: David Hildenbrand <david@redhat.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> 
> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
> index 2c14ad9809da..d0250744507a 100644
> --- a/include/linux/kvm_host.h
> +++ b/include/linux/kvm_host.h
> @@ -162,8 +162,8 @@ int kvm_io_bus_read(struct kvm_vcpu *vcpu, enum kvm_bus bus_idx, gpa_t addr,
>  		    int len, void *val);
>  int kvm_io_bus_register_dev(struct kvm *kvm, enum kvm_bus bus_idx, gpa_t addr,
>  			    int len, struct kvm_io_device *dev);
> -int kvm_io_bus_unregister_dev(struct kvm *kvm, enum kvm_bus bus_idx,
> -			      struct kvm_io_device *dev);
> +void kvm_io_bus_unregister_dev(struct kvm *kvm, enum kvm_bus bus_idx,
> +			       struct kvm_io_device *dev);
>  struct kvm_io_device *kvm_io_bus_get_dev(struct kvm *kvm, enum kvm_bus bus_idx,
>  					 gpa_t addr);
>  
> diff --git a/virt/kvm/eventfd.c b/virt/kvm/eventfd.c
> index a29786dd9522..4d28a9ddbee0 100644
> --- a/virt/kvm/eventfd.c
> +++ b/virt/kvm/eventfd.c
> @@ -870,7 +870,8 @@ kvm_deassign_ioeventfd_idx(struct kvm *kvm, enum kvm_bus bus_idx,
>  			continue;
>  
>  		kvm_io_bus_unregister_dev(kvm, bus_idx, &p->dev);
> -		kvm->buses[bus_idx]->ioeventfd_count--;
> +		if (kvm->buses[bus_idx])
> +			kvm->buses[bus_idx]->ioeventfd_count--;
>  		ioeventfd_release(p);
>  		ret = 0;
>  		break;
> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index 7445566fadc1..ef1aa7f1ed7a 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -728,7 +728,8 @@ static void kvm_destroy_vm(struct kvm *kvm)
>  	spin_unlock(&kvm_lock);
>  	kvm_free_irq_routing(kvm);
>  	for (i = 0; i < KVM_NR_BUSES; i++) {
> -		kvm_io_bus_destroy(kvm->buses[i]);
> +		if (kvm->buses[i])
> +			kvm_io_bus_destroy(kvm->buses[i]);
>  		kvm->buses[i] = NULL;
>  	}
>  	kvm_coalesced_mmio_free(kvm);
> @@ -3476,6 +3477,8 @@ int kvm_io_bus_write(struct kvm_vcpu *vcpu, enum kvm_bus bus_idx, gpa_t addr,
>  	};
>  
>  	bus = srcu_dereference(vcpu->kvm->buses[bus_idx], &vcpu->kvm->srcu);
> +	if (!bus)
> +		return -ENOMEM;
>  	r = __kvm_io_bus_write(vcpu, bus, &range, val);
>  	return r < 0 ? r : 0;
>  }
> @@ -3493,6 +3496,8 @@ int kvm_io_bus_write_cookie(struct kvm_vcpu *vcpu, enum kvm_bus bus_idx,
>  	};
>  
>  	bus = srcu_dereference(vcpu->kvm->buses[bus_idx], &vcpu->kvm->srcu);
> +	if (!bus)
> +		return -ENOMEM;
>  
>  	/* First try the device referenced by cookie. */
>  	if ((cookie >= 0) && (cookie < bus->dev_count) &&
> @@ -3543,6 +3548,8 @@ int kvm_io_bus_read(struct kvm_vcpu *vcpu, enum kvm_bus bus_idx, gpa_t addr,
>  	};
>  
>  	bus = srcu_dereference(vcpu->kvm->buses[bus_idx], &vcpu->kvm->srcu);
> +	if (!bus)
> +		return -ENOMEM;
>  	r = __kvm_io_bus_read(vcpu, bus, &range, val);
>  	return r < 0 ? r : 0;
>  }
> @@ -3555,6 +3562,9 @@ int kvm_io_bus_register_dev(struct kvm *kvm, enum kvm_bus bus_idx, gpa_t addr,
>  	struct kvm_io_bus *new_bus, *bus;
>  
>  	bus = kvm->buses[bus_idx];
> +	if (!bus)
> +		return -ENOMEM;
> +
>  	/* exclude ioeventfd which is limited by maximum fd */
>  	if (bus->dev_count - bus->ioeventfd_count > NR_IOBUS_DEVS - 1)
>  		return -ENOSPC;
> @@ -3574,45 +3584,41 @@ int kvm_io_bus_register_dev(struct kvm *kvm, enum kvm_bus bus_idx, gpa_t addr,
>  }
>  
>  /* Caller must hold slots_lock. */
> -int kvm_io_bus_unregister_dev(struct kvm *kvm, enum kvm_bus bus_idx,
> -			      struct kvm_io_device *dev)
> +void kvm_io_bus_unregister_dev(struct kvm *kvm, enum kvm_bus bus_idx,
> +			       struct kvm_io_device *dev)
>  {
> -	int i, r;
> +	int i;
>  	struct kvm_io_bus *new_bus, *bus;
>  
>  	bus = kvm->buses[bus_idx];
> -
> -	/*
> -	 * It's possible the bus being released before hand. If so,
> -	 * we're done here.
> -	 */
>  	if (!bus)
> -		return 0;
> +		return;
>  
> -	r = -ENOENT;
>  	for (i = 0; i < bus->dev_count; i++)
>  		if (bus->range[i].dev == dev) {
> -			r = 0;
>  			break;
>  		}
>  
> -	if (r)
> -		return r;
> +	if (i == bus->dev_count)
> +		return;
>  
>  	new_bus = kmalloc(sizeof(*bus) + ((bus->dev_count - 1) *
>  			  sizeof(struct kvm_io_range)), GFP_KERNEL);
> -	if (!new_bus)
> -		return -ENOMEM;
> +	if (!new_bus)  {
> +		pr_err("kvm: failed to shrink bus, removing it completely\n");
> +		goto broken;
> +	}
>  
>  	memcpy(new_bus, bus, sizeof(*bus) + i * sizeof(struct kvm_io_range));
>  	new_bus->dev_count--;
>  	memcpy(new_bus->range + i, bus->range + i + 1,
>  	       (new_bus->dev_count - i) * sizeof(struct kvm_io_range));
>  
> +broken:
>  	rcu_assign_pointer(kvm->buses[bus_idx], new_bus);
>  	synchronize_srcu_expedited(&kvm->srcu);
>  	kfree(bus);
> -	return r;
> +	return;
>  }
>  
>  struct kvm_io_device *kvm_io_bus_get_dev(struct kvm *kvm, enum kvm_bus bus_idx,
> @@ -3625,6 +3631,8 @@ struct kvm_io_device *kvm_io_bus_get_dev(struct kvm *kvm, enum kvm_bus bus_idx,
>  	srcu_idx = srcu_read_lock(&kvm->srcu);
>  
>  	bus = srcu_dereference(kvm->buses[bus_idx], &kvm->srcu);
> +	if (!bus)
> +		goto out_unlock;
>  
>  	dev_idx = kvm_io_bus_get_first_dev(bus, addr, 1);
>  	if (dev_idx < 0)
> 

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: FAILED: patch "[PATCH] KVM: kvm_io_bus_unregister_dev() should never fail" failed to apply to 4.10-stable tree
  2017-04-03  8:23 ` David Hildenbrand
@ 2017-04-03 10:35   ` Paolo Bonzini
  0 siblings, 0 replies; 3+ messages in thread
From: Paolo Bonzini @ 2017-04-03 10:35 UTC (permalink / raw)
  To: David Hildenbrand, gregkh; +Cc: cornelia.huck, dvyukov, stable



On 03/04/2017 10:23, David Hildenbrand wrote:
> On 30.03.2017 10:00, gregkh@linuxfoundation.org wrote:
>>
>> The patch below does not apply to the 4.10-stable tree.
>> If someone wants it applied there, or to any other stable or longterm
>> tree, then please email the backport, including the original git commit
>> id to <stable@vger.kernel.org>.
>>
> 
> Hi Greg,
> 
> this commit can be cherry-picked cleanly after cherry picking
> df630b8c1e85 ("KVM: x86: clear bus pointer when destroyed"). Can you
> also apply that commit or should I send a backport of the stable patch,
> including the necessary check from the prerequisite patch?

This extra commit looks good for stable, thanks.

Paolo

>> thanks,
>>
>> greg k-h
>>
>> ------------------ original commit in Linus's tree ------------------
>>
>> From 90db10434b163e46da413d34db8d0e77404cc645 Mon Sep 17 00:00:00 2001
>> From: David Hildenbrand <david@redhat.com>
>> Date: Thu, 23 Mar 2017 18:24:19 +0100
>> Subject: [PATCH] KVM: kvm_io_bus_unregister_dev() should never fail
>>
>> No caller currently checks the return value of
>> kvm_io_bus_unregister_dev(). This is evil, as all callers silently go on
>> freeing their device. A stale reference will remain in the io_bus,
>> getting at least used again, when the iobus gets teared down on
>> kvm_destroy_vm() - leading to use after free errors.
>>
>> There is nothing the callers could do, except retrying over and over
>> again.
>>
>> So let's simply remove the bus altogether, print an error and make
>> sure no one can access this broken bus again (returning -ENOMEM on any
>> attempt to access it).
>>
>> Fixes: e93f8a0f821e ("KVM: convert io_bus to SRCU")
>> Cc: stable@vger.kernel.org # 3.4+
>> Reported-by: Dmitry Vyukov <dvyukov@google.com>
>> Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>
>> Signed-off-by: David Hildenbrand <david@redhat.com>
>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>>
>> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
>> index 2c14ad9809da..d0250744507a 100644
>> --- a/include/linux/kvm_host.h
>> +++ b/include/linux/kvm_host.h
>> @@ -162,8 +162,8 @@ int kvm_io_bus_read(struct kvm_vcpu *vcpu, enum kvm_bus bus_idx, gpa_t addr,
>>  		    int len, void *val);
>>  int kvm_io_bus_register_dev(struct kvm *kvm, enum kvm_bus bus_idx, gpa_t addr,
>>  			    int len, struct kvm_io_device *dev);
>> -int kvm_io_bus_unregister_dev(struct kvm *kvm, enum kvm_bus bus_idx,
>> -			      struct kvm_io_device *dev);
>> +void kvm_io_bus_unregister_dev(struct kvm *kvm, enum kvm_bus bus_idx,
>> +			       struct kvm_io_device *dev);
>>  struct kvm_io_device *kvm_io_bus_get_dev(struct kvm *kvm, enum kvm_bus bus_idx,
>>  					 gpa_t addr);
>>  
>> diff --git a/virt/kvm/eventfd.c b/virt/kvm/eventfd.c
>> index a29786dd9522..4d28a9ddbee0 100644
>> --- a/virt/kvm/eventfd.c
>> +++ b/virt/kvm/eventfd.c
>> @@ -870,7 +870,8 @@ kvm_deassign_ioeventfd_idx(struct kvm *kvm, enum kvm_bus bus_idx,
>>  			continue;
>>  
>>  		kvm_io_bus_unregister_dev(kvm, bus_idx, &p->dev);
>> -		kvm->buses[bus_idx]->ioeventfd_count--;
>> +		if (kvm->buses[bus_idx])
>> +			kvm->buses[bus_idx]->ioeventfd_count--;
>>  		ioeventfd_release(p);
>>  		ret = 0;
>>  		break;
>> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
>> index 7445566fadc1..ef1aa7f1ed7a 100644
>> --- a/virt/kvm/kvm_main.c
>> +++ b/virt/kvm/kvm_main.c
>> @@ -728,7 +728,8 @@ static void kvm_destroy_vm(struct kvm *kvm)
>>  	spin_unlock(&kvm_lock);
>>  	kvm_free_irq_routing(kvm);
>>  	for (i = 0; i < KVM_NR_BUSES; i++) {
>> -		kvm_io_bus_destroy(kvm->buses[i]);
>> +		if (kvm->buses[i])
>> +			kvm_io_bus_destroy(kvm->buses[i]);
>>  		kvm->buses[i] = NULL;
>>  	}
>>  	kvm_coalesced_mmio_free(kvm);
>> @@ -3476,6 +3477,8 @@ int kvm_io_bus_write(struct kvm_vcpu *vcpu, enum kvm_bus bus_idx, gpa_t addr,
>>  	};
>>  
>>  	bus = srcu_dereference(vcpu->kvm->buses[bus_idx], &vcpu->kvm->srcu);
>> +	if (!bus)
>> +		return -ENOMEM;
>>  	r = __kvm_io_bus_write(vcpu, bus, &range, val);
>>  	return r < 0 ? r : 0;
>>  }
>> @@ -3493,6 +3496,8 @@ int kvm_io_bus_write_cookie(struct kvm_vcpu *vcpu, enum kvm_bus bus_idx,
>>  	};
>>  
>>  	bus = srcu_dereference(vcpu->kvm->buses[bus_idx], &vcpu->kvm->srcu);
>> +	if (!bus)
>> +		return -ENOMEM;
>>  
>>  	/* First try the device referenced by cookie. */
>>  	if ((cookie >= 0) && (cookie < bus->dev_count) &&
>> @@ -3543,6 +3548,8 @@ int kvm_io_bus_read(struct kvm_vcpu *vcpu, enum kvm_bus bus_idx, gpa_t addr,
>>  	};
>>  
>>  	bus = srcu_dereference(vcpu->kvm->buses[bus_idx], &vcpu->kvm->srcu);
>> +	if (!bus)
>> +		return -ENOMEM;
>>  	r = __kvm_io_bus_read(vcpu, bus, &range, val);
>>  	return r < 0 ? r : 0;
>>  }
>> @@ -3555,6 +3562,9 @@ int kvm_io_bus_register_dev(struct kvm *kvm, enum kvm_bus bus_idx, gpa_t addr,
>>  	struct kvm_io_bus *new_bus, *bus;
>>  
>>  	bus = kvm->buses[bus_idx];
>> +	if (!bus)
>> +		return -ENOMEM;
>> +
>>  	/* exclude ioeventfd which is limited by maximum fd */
>>  	if (bus->dev_count - bus->ioeventfd_count > NR_IOBUS_DEVS - 1)
>>  		return -ENOSPC;
>> @@ -3574,45 +3584,41 @@ int kvm_io_bus_register_dev(struct kvm *kvm, enum kvm_bus bus_idx, gpa_t addr,
>>  }
>>  
>>  /* Caller must hold slots_lock. */
>> -int kvm_io_bus_unregister_dev(struct kvm *kvm, enum kvm_bus bus_idx,
>> -			      struct kvm_io_device *dev)
>> +void kvm_io_bus_unregister_dev(struct kvm *kvm, enum kvm_bus bus_idx,
>> +			       struct kvm_io_device *dev)
>>  {
>> -	int i, r;
>> +	int i;
>>  	struct kvm_io_bus *new_bus, *bus;
>>  
>>  	bus = kvm->buses[bus_idx];
>> -
>> -	/*
>> -	 * It's possible the bus being released before hand. If so,
>> -	 * we're done here.
>> -	 */
>>  	if (!bus)
>> -		return 0;
>> +		return;
>>  
>> -	r = -ENOENT;
>>  	for (i = 0; i < bus->dev_count; i++)
>>  		if (bus->range[i].dev == dev) {
>> -			r = 0;
>>  			break;
>>  		}
>>  
>> -	if (r)
>> -		return r;
>> +	if (i == bus->dev_count)
>> +		return;
>>  
>>  	new_bus = kmalloc(sizeof(*bus) + ((bus->dev_count - 1) *
>>  			  sizeof(struct kvm_io_range)), GFP_KERNEL);
>> -	if (!new_bus)
>> -		return -ENOMEM;
>> +	if (!new_bus)  {
>> +		pr_err("kvm: failed to shrink bus, removing it completely\n");
>> +		goto broken;
>> +	}
>>  
>>  	memcpy(new_bus, bus, sizeof(*bus) + i * sizeof(struct kvm_io_range));
>>  	new_bus->dev_count--;
>>  	memcpy(new_bus->range + i, bus->range + i + 1,
>>  	       (new_bus->dev_count - i) * sizeof(struct kvm_io_range));
>>  
>> +broken:
>>  	rcu_assign_pointer(kvm->buses[bus_idx], new_bus);
>>  	synchronize_srcu_expedited(&kvm->srcu);
>>  	kfree(bus);
>> -	return r;
>> +	return;
>>  }
>>  
>>  struct kvm_io_device *kvm_io_bus_get_dev(struct kvm *kvm, enum kvm_bus bus_idx,
>> @@ -3625,6 +3631,8 @@ struct kvm_io_device *kvm_io_bus_get_dev(struct kvm *kvm, enum kvm_bus bus_idx,
>>  	srcu_idx = srcu_read_lock(&kvm->srcu);
>>  
>>  	bus = srcu_dereference(kvm->buses[bus_idx], &kvm->srcu);
>> +	if (!bus)
>> +		goto out_unlock;
>>  
>>  	dev_idx = kvm_io_bus_get_first_dev(bus, addr, 1);
>>  	if (dev_idx < 0)
>>

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2017-04-03 10:36 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-30  8:00 FAILED: patch "[PATCH] KVM: kvm_io_bus_unregister_dev() should never fail" failed to apply to 4.10-stable tree gregkh
2017-04-03  8:23 ` David Hildenbrand
2017-04-03 10:35   ` Paolo Bonzini

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.