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

[Vishnu Dasa]

On Mon, Jul 20, 2026 at 5:09 AM Abdun Nihaal <nihaal@xxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxx
> Signed-off-by: Abdun Nihaal <nihaal@xxxxxxxxxxxxxx>
> ---
> 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
>

Attachment: smime.p7s
Description: S/MIME Cryptographic Signature