[4/4] KVM: PPC: Book3S HV: Remove redundant parameter from resize_hpt_release()

Message ID 1511973506-65683-5-git-send-email-spopovyc@redhat.com
State New
Headers show
Series
  • Fix use after free in HPT resizing code and related minor improvements
Related show

Commit Message

Serhii Popovych Nov. 29, 2017, 4:38 p.m.
There is no need to pass it explicitly from the caller:
struct kvm_resize_hpt already contains it.

Additional benefit from this change is that BUG_ON()
assertion now checks that mutex is held on kvm instance
associated with resize structure we going to release.

Also kill check for resize being NULL to make code
simpler and we called with resize != NULL in all
places except kvm_vm_ioctl_resize_hpt_commit().

Signed-off-by: Serhii Popovych <spopovyc@redhat.com>
---
 arch/powerpc/kvm/book3s_64_mmu_hv.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

Comments

David Gibson Nov. 30, 2017, 3:53 a.m. | #1
On Wed, Nov 29, 2017 at 11:38:26AM -0500, Serhii Popovych wrote:
> There is no need to pass it explicitly from the caller:
> struct kvm_resize_hpt already contains it.
> 
> Additional benefit from this change is that BUG_ON()
> assertion now checks that mutex is held on kvm instance
> associated with resize structure we going to release.
> 
> Also kill check for resize being NULL to make code
> simpler and we called with resize != NULL in all
> places except kvm_vm_ioctl_resize_hpt_commit().
> 
> Signed-off-by: Serhii Popovych <spopovyc@redhat.com>

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

> ---
>  arch/powerpc/kvm/book3s_64_mmu_hv.c | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/arch/powerpc/kvm/book3s_64_mmu_hv.c b/arch/powerpc/kvm/book3s_64_mmu_hv.c
> index 690f061..a74a0ad 100644
> --- a/arch/powerpc/kvm/book3s_64_mmu_hv.c
> +++ b/arch/powerpc/kvm/book3s_64_mmu_hv.c
> @@ -1415,12 +1415,11 @@ static void resize_hpt_pivot(struct kvm_resize_hpt *resize)
>  	resize_hpt_debug(resize, "resize_hpt_pivot() done\n");
>  }
>  
> -static void resize_hpt_release(struct kvm *kvm, struct kvm_resize_hpt *resize)
> +static void resize_hpt_release(struct kvm_resize_hpt *resize)
>  {
> -	BUG_ON(!mutex_is_locked(&kvm->lock));
> +	struct kvm *kvm = resize->kvm;
>  
> -	if (!resize)
> -		return;
> +	BUG_ON(!mutex_is_locked(&kvm->lock));
>  
>  	if (resize->error != -EBUSY) {
>  		kvmppc_free_hpt(&resize->hpt);
> @@ -1469,7 +1468,7 @@ static void resize_hpt_prepare_work(struct work_struct *work)
>  	resize->error = err;
>  
>  	if (kvm->arch.resize_hpt != resize)
> -		resize_hpt_release(kvm, resize);
> +		resize_hpt_release(resize);
>  
>  	mutex_unlock(&kvm->lock);
>  }
> @@ -1499,13 +1498,13 @@ long kvm_vm_ioctl_resize_hpt_prepare(struct kvm *kvm,
>  			if (ret == -EBUSY)
>  				ret = 100; /* estimated time in ms */
>  			else if (ret)
> -				resize_hpt_release(kvm, resize);
> +				resize_hpt_release(resize);
>  
>  			goto out;
>  		}
>  
>  		/* not suitable, cancel it */
> -		resize_hpt_release(kvm, resize);
> +		resize_hpt_release(resize);
>  	}
>  
>  	ret = 0;
> @@ -1590,7 +1589,8 @@ long kvm_vm_ioctl_resize_hpt_commit(struct kvm *kvm,
>  	kvm->arch.mmu_ready = 1;
>  	smp_mb();
>  out_no_hpt:
> -	resize_hpt_release(kvm, resize);
> +	if (resize)
> +		resize_hpt_release(resize);
>  	mutex_unlock(&kvm->lock);
>  	return ret;
>  }

Patch

diff --git a/arch/powerpc/kvm/book3s_64_mmu_hv.c b/arch/powerpc/kvm/book3s_64_mmu_hv.c
index 690f061..a74a0ad 100644
--- a/arch/powerpc/kvm/book3s_64_mmu_hv.c
+++ b/arch/powerpc/kvm/book3s_64_mmu_hv.c
@@ -1415,12 +1415,11 @@  static void resize_hpt_pivot(struct kvm_resize_hpt *resize)
 	resize_hpt_debug(resize, "resize_hpt_pivot() done\n");
 }
 
-static void resize_hpt_release(struct kvm *kvm, struct kvm_resize_hpt *resize)
+static void resize_hpt_release(struct kvm_resize_hpt *resize)
 {
-	BUG_ON(!mutex_is_locked(&kvm->lock));
+	struct kvm *kvm = resize->kvm;
 
-	if (!resize)
-		return;
+	BUG_ON(!mutex_is_locked(&kvm->lock));
 
 	if (resize->error != -EBUSY) {
 		kvmppc_free_hpt(&resize->hpt);
@@ -1469,7 +1468,7 @@  static void resize_hpt_prepare_work(struct work_struct *work)
 	resize->error = err;
 
 	if (kvm->arch.resize_hpt != resize)
-		resize_hpt_release(kvm, resize);
+		resize_hpt_release(resize);
 
 	mutex_unlock(&kvm->lock);
 }
@@ -1499,13 +1498,13 @@  long kvm_vm_ioctl_resize_hpt_prepare(struct kvm *kvm,
 			if (ret == -EBUSY)
 				ret = 100; /* estimated time in ms */
 			else if (ret)
-				resize_hpt_release(kvm, resize);
+				resize_hpt_release(resize);
 
 			goto out;
 		}
 
 		/* not suitable, cancel it */
-		resize_hpt_release(kvm, resize);
+		resize_hpt_release(resize);
 	}
 
 	ret = 0;
@@ -1590,7 +1589,8 @@  long kvm_vm_ioctl_resize_hpt_commit(struct kvm *kvm,
 	kvm->arch.mmu_ready = 1;
 	smp_mb();
 out_no_hpt:
-	resize_hpt_release(kvm, resize);
+	if (resize)
+		resize_hpt_release(resize);
 	mutex_unlock(&kvm->lock);
 	return ret;
 }