[openib-general] [PATCH] Fix panic and memory leak in SA Query.

[Krishna Kumar]

Hi Roland,

I agree with this. BTW, can't the release handler execute before the
(I know, quirky race, but interrupts ...) :
	} else
		*sa_query = &query->sa_query

and free up the memory ? Do you want send_mad() to return a copy of
the sa_query (*copy = *query) before it actually sends on the wire ?
The callers can use this on success case to return sa_query and id.

- KK

On Fri, 5 Nov 2004, Roland Dreier wrote:

> Sorry, this and the follow-up patch are wrong.  The if the send
> succeeds then we can't free the query structure until the query
> finishes up.  (The query will be freed in the appropriate ->release
> method in this case).
>
> You are right that there is a memory leak though.  I fixed it like
> this:
>
> Index: infiniband/core/sa_query.c
> ===================================================================
> --- infiniband/core/sa_query.c	(revision 1166)
> +++ infiniband/core/sa_query.c	(working copy)
> @@ -500,6 +500,7 @@
>
>  static void ib_sa_path_rec_release(struct ib_sa_query *sa_query)
>  {
> +	kfree(sa_query->mad);
>  	kfree(container_of(sa_query, struct ib_sa_path_query, sa_query));
>  }
>
> @@ -544,11 +545,12 @@
>  		rec, query->sa_query.mad->data);
>
>  	ret = send_mad(&query->sa_query, timeout_ms);
> -	if (ret)
> +	if (ret) {
> +		kfree(query->sa_query.mad);
>  		kfree(query);
> +	} else
> +		*sa_query = &query->sa_query;
>
> -	*sa_query = &query->sa_query;
> -
>  	return ret ? ret : query->sa_query.id;
>  }
>  EXPORT_SYMBOL(ib_sa_path_rec_get);
> @@ -572,6 +574,7 @@
>
>  static void ib_sa_mcmember_rec_release(struct ib_sa_query *sa_query)
>  {
> +	kfree(sa_query->mad);
>  	kfree(container_of(sa_query, struct ib_sa_mcmember_query, sa_query));
>  }
>
> @@ -617,11 +620,12 @@
>  		rec, query->sa_query.mad->data);
>
>  	ret = send_mad(&query->sa_query, timeout_ms);
> -	if (ret)
> +	if (ret) {
> +		kfree(query->sa_query.mad);
>  		kfree(query);
> +	} else
> +		*sa_query = &query->sa_query;
>
> -	*sa_query = &query->sa_query;
> -
>  	return ret ? ret : query->sa_query.id;
>  }
>  EXPORT_SYMBOL(ib_sa_mcmember_rec_query);
>
>