KVM: x86/xen: Fix eventfd error handling in kvm_xen_eventfd_assign()

Commit Message

Eiichi Tsukata Oct. 28, 2022, 9:26 a.m. UTC

Should not call eventfd_ctx_put() in case of error.

Fixes: 2fd6df2f2b47 ("KVM: x86/xen: intercept EVTCHNOP_send from guests")
Reported-by: [e-mail]
Signed-off-by: Eiichi Tsukata <[e-mail]>
---
 arch/x86/kvm/xen.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Paolo Bonzini Oct. 28, 2022, 10:47 a.m. UTC | #1

On 10/28/22 11:26, Eiichi Tsukata wrote:
> Should not call eventfd_ctx_put() in case of error.
> 
> Fixes: 2fd6df2f2b47 ("KVM: x86/xen: intercept EVTCHNOP_send from guests")
> Reported-by: [e-mail]
> Signed-off-by: Eiichi Tsukata <[e-mail]>
> ---
>   arch/x86/kvm/xen.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/x86/kvm/xen.c b/arch/x86/kvm/xen.c
> index 93c628d3e3a9..a357994982c6 100644
> --- a/arch/x86/kvm/xen.c
> +++ b/arch/x86/kvm/xen.c
> @@ -1716,7 +1716,7 @@ static int kvm_xen_eventfd_assign(struct kvm *kvm,
>   	if (ret == -ENOSPC)
>   		ret = -EEXIST;
>   out:
> -	if (eventfd)
> +	if (eventfd && !IS_ERR(eventfd))
>   		eventfd_ctx_put(eventfd);
>   	kfree(evtchnfd);
>   	return ret;

Slightly more verbose, but cleaner:

diff --git a/arch/x86/kvm/xen.c b/arch/x86/kvm/xen.c
index 6714bbdbedf3..2dae413bd62a 100644
--- a/arch/x86/kvm/xen.c
+++ b/arch/x86/kvm/xen.c
@@ -1666,18 +1666,18 @@ static int kvm_xen_eventfd_assign(struct kvm *kvm,
  	case EVTCHNSTAT_ipi:
  		/* IPI  must map back to the same port# */
  		if (data->u.evtchn.deliver.port.port != data->u.evtchn.send_port)
-			goto out; /* -EINVAL */
+			goto out_noeventfd; /* -EINVAL */
  		break;
  
  	case EVTCHNSTAT_interdomain:
  		if (data->u.evtchn.deliver.port.port) {
  			if (data->u.evtchn.deliver.port.port >= max_evtchn_port(kvm))
-				goto out; /* -EINVAL */
+				goto out_noeventfd; /* -EINVAL */
  		} else {
  			eventfd = eventfd_ctx_fdget(data->u.evtchn.deliver.eventfd.fd);
  			if (IS_ERR(eventfd)) {
  				ret = PTR_ERR(eventfd);
-				goto out;
+				goto out_noeventfd;
  			}
  		}
  		break;
@@ -1717,6 +1717,7 @@ static int kvm_xen_eventfd_assign(struct kvm *kvm,
  out:
	if (eventfd)
  		eventfd_ctx_put(eventfd);
+out_noeventfd:
  	kfree(evtchnfd);
  	return ret;
  }

Only the last goto has to be changed in order to fix the bug, the
others are only needed to respect the LIFO order of the unwinding
labels.

Paolo

Patch

diff --git a/arch/x86/kvm/xen.c b/arch/x86/kvm/xen.c
index 93c628d3e3a9..a357994982c6 100644
--- a/arch/x86/kvm/xen.c
+++ b/arch/x86/kvm/xen.c
@@ -1716,7 +1716,7 @@  static int kvm_xen_eventfd_assign(struct kvm *kvm,
 	if (ret == -ENOSPC)
 		ret = -EEXIST;
 out:
-	if (eventfd)
+	if (eventfd && !IS_ERR(eventfd))
 		eventfd_ctx_put(eventfd);
 	kfree(evtchnfd);
 	return ret;