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

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

Posted by Abdun Nihaal 2 days, 12 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.

v1->v2:
- Moved the new_subscription_id assignment to the point before the
  if/else to simplify the code, as suggested by Vishnu Dasa.

v1 link: https://lore.kernel.org/all/CAF+opq15TtOnHaLvYr0TH18sCDEPbTYRTsFZMcyjgvvLd_ARkw@mail.gmail.com/

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

diff --git a/drivers/misc/vmw_vmci/vmci_event.c b/drivers/misc/vmw_vmci/vmci_event.c
index fffe068a26eb..c3ef3b98c432 100644
--- a/drivers/misc/vmw_vmci/vmci_event.c
+++ b/drivers/misc/vmw_vmci/vmci_event.c
@@ -179,16 +179,16 @@ int vmci_event_subscribe(u32 event,
 		}
 	}
 
+	*new_subscription_id = sub->id;
 	if (have_new_id) {
 		list_add_rcu(&sub->node, &subscriber_array[event]);
 		retval = VMCI_SUCCESS;
 	} else {
+		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 v2] misc: vmc_vmci: Fix potential memory leak in vmci_event_subscribe()

Posted by Vishnu Dasa 2 days, 2 hours ago

On Wed, Jul 22, 2026 at 3:12 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>

Thanks!

Acked-by: Vishnu Dasa <vishnu.dasa@broadcom.com>

> ---
> Compile tested only. Issue found using static analysis.
>
> v1->v2:
> - Moved the new_subscription_id assignment to the point before the
>   if/else to simplify the code, as suggested by Vishnu Dasa.
>
> v1 link: https://lore.kernel.org/all/CAF+opq15TtOnHaLvYr0TH18sCDEPbTYRTsFZMcyjgvvLd_ARkw@mail.gmail.com/
>
>  drivers/misc/vmw_vmci/vmci_event.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/misc/vmw_vmci/vmci_event.c b/drivers/misc/vmw_vmci/vmci_event.c
> index fffe068a26eb..c3ef3b98c432 100644
> --- a/drivers/misc/vmw_vmci/vmci_event.c
> +++ b/drivers/misc/vmw_vmci/vmci_event.c
> @@ -179,16 +179,16 @@ int vmci_event_subscribe(u32 event,
>                 }
>         }
>
> +       *new_subscription_id = sub->id;
>         if (have_new_id) {
>                 list_add_rcu(&sub->node, &subscriber_array[event]);
>                 retval = VMCI_SUCCESS;
>         } else {
> +               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
>