From: Roman Kisel <[email protected]> Sent: Monday, July 14, 2025 3:16 PM > > The early failure path in __vmbus_establish_gpadl() doesn't deallocate > msginfo if the buffer fails to decrypt. > > Fix the leak by breaking out the cleanup code into a separate function > and calling it where required. > > Fixes: d4dccf353db80 ("Drivers: hv: vmbus: Mark vmbus ring buffer visible to > host in Isolation VM") > Reported-by: Michael Kelly <[email protected]>
s/Kelly/Kelley/ > Closes: > https://lore.kernel.org/linux-hyperv/sn6pr02mb41573796f9787f67e0e97049d4...@sn6pr02mb4157.namprd02.prod.outlook.com/ > > Signed-off-by: Roman Kisel <[email protected]> > --- > drivers/hv/channel.c | 32 ++++++++++++++++++++++---------- > 1 file changed, 22 insertions(+), 10 deletions(-) > > diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c > index 051eeba800f2..0eb300b940db 100644 > --- a/drivers/hv/channel.c > +++ b/drivers/hv/channel.c > @@ -409,6 +409,25 @@ static int create_gpadl_header(enum hv_gpadl_type type, > void *kbuffer, > return 0; > } > > +static void vmbus_free_channel_msginfo(struct vmbus_channel_msginfo *msginfo) > +{ > + unsigned long flags; > + struct vmbus_channel_msginfo *submsginfo, *tmp; > + > + if (!msginfo) > + return; > + > + spin_lock_irqsave(&vmbus_connection.channelmsg_lock, flags); > + list_del(&msginfo->msglistentry); > + spin_unlock_irqrestore(&vmbus_connection.channelmsg_lock, flags); > + list_for_each_entry_safe(submsginfo, tmp, &msginfo->submsglist, > + msglistentry) { > + kfree(submsginfo); > + } > + > + kfree(msginfo); > +} > + > /* > * __vmbus_establish_gpadl - Establish a GPADL for a buffer or ringbuffer > * > @@ -428,7 +447,7 @@ static int __vmbus_establish_gpadl(struct vmbus_channel > *channel, > struct vmbus_channel_gpadl_header *gpadlmsg; > struct vmbus_channel_gpadl_body *gpadl_body; > struct vmbus_channel_msginfo *msginfo = NULL; > - struct vmbus_channel_msginfo *submsginfo, *tmp; > + struct vmbus_channel_msginfo *submsginfo; > struct list_head *curr; > u32 next_gpadl_handle; > unsigned long flags; > @@ -458,6 +477,7 @@ static int __vmbus_establish_gpadl(struct vmbus_channel > *channel, > dev_warn(&channel->device_obj->device, > "Failed to set host visibility for new GPADL > %d.\n", > ret); > + vmbus_free_channel_msginfo(msginfo); I don't think this works. At this point, msginfo has not been added to the global vmbus_connection.chn_msg_list. vmbus_free_channel_msginfo() will try to remove it from that list using list_del(), and will fault on a NULL pointer. > return ret; > } > } > @@ -531,15 +551,7 @@ static int __vmbus_establish_gpadl(struct vmbus_channel > *channel, > > > cleanup: > - spin_lock_irqsave(&vmbus_connection.channelmsg_lock, flags); > - list_del(&msginfo->msglistentry); > - spin_unlock_irqrestore(&vmbus_connection.channelmsg_lock, flags); > - list_for_each_entry_safe(submsginfo, tmp, &msginfo->submsglist, > - msglistentry) { > - kfree(submsginfo); > - } > - > - kfree(msginfo); > + vmbus_free_channel_msginfo(msginfo); > > if (ret) { > /* > -- > 2.43.0
