[ofa-general] [PATCH] IB/core: fix null pointer dereference in local_completions()

Ralph Campbell ralph.campbell at qlogic.com
Wed Feb 25 17:43:58 PST 2009


On Wed, 2009-02-25 at 17:22 -0800, Sean Hefty wrote:
> >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()
> >but recv == 0 so kmem_cache_free() is called with a
> >NULL pointer.
> >
> >Also, since recv isn't reinitialized each time through the loop,
> >it can cause a memory leak if recv should have been zero.
> >
> >Signed-off-by: Ralph Campbell <ralph.campbell at qlogic.com>
> >
> >diff --git a/drivers/infiniband/core/mad.c b/drivers/infiniband/core/mad.c
> >index 5c54fc2..8388e5e 100644
> >--- a/drivers/infiniband/core/mad.c
> >+++ b/drivers/infiniband/core/mad.c
> >@@ -2356,7 +2356,7 @@ 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;
> >+	int recv;
> 
> With this change, I think it would be better to rename the 'recv' flag.  The
> logic itself looks correct to me.
> 
> - Sean

OK, how about "free" or "free_mad"?




More information about the general mailing list