[openib-general] Re: [PATCH] core: fix user_mad memory leaks on timeout

[Hal Rosenstock]

On Thu, 2005-12-08 at 08:51, Michael S. Tsirkin wrote:
> Dont leak packet if it had a timeout.
> Dont leak timeout mad if queue_packet fails.
> 
> Signed-off-by: Jack Morgenstein <jackm at mellanox.co.il>
> Signed-off-by: Michael S. Tsirkin <mst at mellanox.co.il>
> 
> Index: openib/drivers/infiniband/core/user_mad.c
> ===================================================================
> --- openib.orig/drivers/infiniband/core/user_mad.c	2005-12-08 15:40:41.000000000 +0200
> +++ openib/drivers/infiniband/core/user_mad.c	2005-12-08 15:40:28.000000000 +0200
> @@ -197,8 +197,8 @@ static void send_handler(struct ib_mad_a
>  		memcpy(timeout->mad.data, packet->mad.data,
>  		       sizeof (struct ib_mad_hdr));
>  
> -		if (!queue_packet(file, agent, timeout))
> -				return;
> +		if (queue_packet(file, agent, timeout))
> +			kfree(timeout);

Yes, there appears to be a memory leak here but I don't think this fix
is quite right as it has lost the return when the queue_packet succeeds.
Isn't that still needed ?

		if (!queue_packet(file, agent, timeout))
			return;
		kfree(timeout);

>  	}
>  out:
>  	kfree(packet);

Another point: on either failure to allocate the timeout MAD or failure
to queue the timeout MAD, is simply throwing this away sufficient ? It
seems to me that if this occurs, then the contract is broken and the
client still needs to worry about its own timeout.

-- Hal