2017-03-13 22:58:22

by kys

Subject: [PATCH 1/1] Drivers: hv: vmbus: Don't leak channel ids

From: K. Y. Srinivasan <[email protected]>

If we cannot allocate memory for the channel, free the relid
associated with the channel.

Signed-off-by: K. Y. Srinivasan <[email protected]>
Cc: <[email protected]>
---
drivers/hv/channel_mgmt.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
index e1a3ae4..0a85246 100644
--- a/drivers/hv/channel_mgmt.c
+++ b/drivers/hv/channel_mgmt.c
@@ -802,6 +802,7 @@ static void vmbus_onoffer(struct vmbus_channel_message_header *hdr)
/* Allocate the channel object and save this offer. */
newchannel = alloc_channel();
if (!newchannel) {
+ vmbus_release_relid(offer->child_relid);
pr_err("Unable to allocate channel object\n");
return;
}
--
1.7.1

2017-03-14 12:49:32

by Dexuan Cui

Subject: RE: [PATCH 1/1] Drivers: hv: vmbus: Don't leak channel ids

> From: [email protected] [mailto:linux-kernel-
> [email protected]] On Behalf Of [email protected]
>
> diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
> index e1a3ae4..0a85246 100644
> --- a/drivers/hv/channel_mgmt.c
> +++ b/drivers/hv/channel_mgmt.c
> @@ -802,6 +802,7 @@ static void vmbus_onoffer(struct
> vmbus_channel_message_header *hdr)
> /* Allocate the channel object and save this offer. */
> newchannel = alloc_channel();
> if (!newchannel) {
> + vmbus_release_relid(offer->child_relid);
> pr_err("Unable to allocate channel object\n");
> return;
> }

The patch seems good.

BTW, vmbus_onoffer -> alloc_channel is only called in the workqueue
context, so maybe we should change the atomic kzalloc in alloc_channel
to GFP_KERNEL?

Thanks,
-- Dexuan

2017-03-14 14:34:26

by KY Srinivasan

Subject: RE: [PATCH 1/1] Drivers: hv: vmbus: Don't leak channel ids

> -----Original Message-----
> From: Dexuan Cui
> Sent: Tuesday, March 14, 2017 5:49 AM
> To: KY Srinivasan <[email protected]>; [email protected]; linux-
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]
> Cc: [email protected]
> Subject: RE: [PATCH 1/1] Drivers: hv: vmbus: Don't leak channel ids
>
> > From: [email protected] [mailto:linux-kernel-
> > [email protected]] On Behalf Of [email protected]
> >
> > diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
> > index e1a3ae4..0a85246 100644
> > --- a/drivers/hv/channel_mgmt.c
> > +++ b/drivers/hv/channel_mgmt.c
> > @@ -802,6 +802,7 @@ static void vmbus_onoffer(struct
> > vmbus_channel_message_header *hdr)
> > /* Allocate the channel object and save this offer. */
> > newchannel = alloc_channel();
> > if (!newchannel) {
> > + vmbus_release_relid(offer->child_relid);
> > pr_err("Unable to allocate channel object\n");
> > return;
> > }
>
> The patch seems good.
>
> BTW, vmbus_onoffer -> alloc_channel is only called in the workqueue
> context, so maybe we should change the atomic kzalloc in alloc_channel
> to GFP_KERNEL?

No; this will introduce potential reordering of execution. I am working on rescind handling
where, we need to ensure temporal ordering of events until the channel is created.

K. Y
>
> Thanks,
> -- Dexuan