[v1] misc: vmc_vmci: Fix potential memory leak in vmci_event_subscribe()

[PATCH] misc: vmc_vmci: Fix potential memory leak in vmci_event_subscribe()

Posted by Abdun Nihaal 4 days, 13 hours ago

The memory allocated for struct vmci_subscription (sub) is not freed
in the error path when have_new_id is false. Fix that by adding a
kfree() call, and moving the read of sub->id to a point before freeing.

Fixes: 1d990201f9bb ("VMCI: event handling implementation.")
Cc: stable@vger.kernel.org
Signed-off-by: Abdun Nihaal <nihaal@cse.iitm.ac.in>
---
Compile tested only. Issue found using static analysis.

 drivers/misc/vmw_vmci/vmci_event.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/misc/vmw_vmci/vmci_event.c b/drivers/misc/vmw_vmci/vmci_event.c
index fffe068a26eb..a7f7363c577f 100644
--- a/drivers/misc/vmw_vmci/vmci_event.c
+++ b/drivers/misc/vmw_vmci/vmci_event.c
@@ -181,14 +181,15 @@ int vmci_event_subscribe(u32 event,
 
 	if (have_new_id) {
 		list_add_rcu(&sub->node, &subscriber_array[event]);
+		*new_subscription_id = sub->id;
 		retval = VMCI_SUCCESS;
 	} else {
+		*new_subscription_id = sub->id;
+		kfree(sub);
 		retval = VMCI_ERROR_NO_RESOURCES;
 	}
 
 	mutex_unlock(&subscriber_mutex);
-
-	*new_subscription_id = sub->id;
 	return retval;
 }
 EXPORT_SYMBOL_GPL(vmci_event_subscribe);
-- 
2.43.0

Re: [PATCH] misc: vmc_vmci: Fix potential memory leak in vmci_event_subscribe()

Posted by Vishnu Dasa 4 days, 2 hours ago

On Mon, Jul 20, 2026 at 5:09 AM Abdun Nihaal <nihaal@cse.iitm.ac.in> wrote:
>
> The memory allocated for struct vmci_subscription (sub) is not freed
> in the error path when have_new_id is false. Fix that by adding a
> kfree() call, and moving the read of sub->id to a point before freeing.
>
> Fixes: 1d990201f9bb ("VMCI: event handling implementation.")
> Cc: stable@vger.kernel.org
> Signed-off-by: Abdun Nihaal <nihaal@cse.iitm.ac.in>
> ---
> Compile tested only. Issue found using static analysis.
>
>  drivers/misc/vmw_vmci/vmci_event.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/misc/vmw_vmci/vmci_event.c b/drivers/misc/vmw_vmci/vmci_event.c
> index fffe068a26eb..a7f7363c577f 100644
> --- a/drivers/misc/vmw_vmci/vmci_event.c
> +++ b/drivers/misc/vmw_vmci/vmci_event.c
> @@ -181,14 +181,15 @@ int vmci_event_subscribe(u32 event,
>
>         if (have_new_id) {
>                 list_add_rcu(&sub->node, &subscriber_array[event]);
> +               *new_subscription_id = sub->id;

sub->id is already determined before the if/else. So, this can safely be
moved to before the if/else instead of duplicating it.

The other option is to move it back to its original place and add kfree with
a check for retval is not VMCI_SUCCESS, just before returning. This will also
have a smaller critical section.

>                 retval = VMCI_SUCCESS;
>         } else {
> +               *new_subscription_id = sub->id;
> +               kfree(sub);
>                 retval = VMCI_ERROR_NO_RESOURCES;
>         }
>
>         mutex_unlock(&subscriber_mutex);
> -
> -       *new_subscription_id = sub->id;
>         return retval;
>  }
>  EXPORT_SYMBOL_GPL(vmci_event_subscribe);
> --
> 2.43.0
>