[ofa-general] Possible memory leak and null pointer dereference in local_completions()
Ralph Campbell
ralph.campbell at qlogic.com
Tue Feb 3 11:26:12 PST 2009
I was doing some tests with different MAD packets and
then reading the infiniband/core/mad.c code.
handle_outgoing_dr_smp() can queue a struct ib_mad_local_private *local
on the mad_agent_priv->local_work work queue with
local->mad_priv == NULL if device->process_mad() returns
IB_MAD_RESULT_SUCCESS | IB_MAD_RESULT_REPLY and
(!ib_response_mad(&mad_priv->mad.mad) ||
!mad_agent_priv->agent.recv_handler).
In this case, local_completions() will be called with
local->mad_priv == NULL. The code does check for this
case and skips calling recv_mad_agent->agent.recv_handler().
This means recv == 0 so kmem_cache_free() is called with a
NULL pointer.
Even if local->mad_priv != NULL, I don't see how local->mad_priv
is freed when recv == 1. Thus, it appears to be a memory leak.
So, I'm proposing the following patch:
diff --git a/drivers/infiniband/core/mad.c b/drivers/infiniband/core/mad.c
index 5c54fc2..93d80e5 100644
--- a/drivers/infiniband/core/mad.c
+++ b/drivers/infiniband/core/mad.c
@@ -2356,7 +2356,6 @@ static void local_completions(struct work_struct *work)
struct ib_mad_local_private *local;
struct ib_mad_agent_private *recv_mad_agent;
unsigned long flags;
- int recv = 0;
struct ib_wc wc;
struct ib_mad_send_wc mad_send_wc;
@@ -2377,7 +2376,6 @@ static void local_completions(struct work_struct *work)
goto local_send_completion;
}
- recv = 1;
/*
* Defined behavior is to complete response
* before request
@@ -2422,7 +2420,7 @@ local_send_completion:
spin_lock_irqsave(&mad_agent_priv->lock, flags);
atomic_dec(&mad_agent_priv->refcount);
- if (!recv)
+ if (local->mad_priv)
kmem_cache_free(ib_mad_cache, local->mad_priv);
kfree(local);
}
More information about the general
mailing list