The deference occurs in the non-IS_ERR() path. Here is the patched code:
vah = ib_create_ah(pd, attr);
if (IS_ERR(vah)) {
kfree(ah);
ah = (struct ipoib_ah *)vah;
} else {
ah->ah = vah; <------------------ defer w/o free
ipoib_dbg(netdev_priv(dev), "Created ah %p\n", ah->ah);
}
I agree that perhaps the code should just return a cast of vah in the IS_ERR()
case so it's clearer?
Mike
> -----Original Message-----
> From: Hefty, Sean [mailto:[email protected]]
> Sent: Monday, November 21, 2011 11:53 AM
> To: Mike Marciniszyn; [email protected]
> Cc: [email protected]
> Subject: RE: [PATCH] IB/ipoib: Prevent hung task or softlockup
> processing multicast response
>
> > --- a/drivers/infiniband/ulp/ipoib/ipoib_ib.c
> > +++ b/drivers/infiniband/ulp/ipoib/ipoib_ib.c
> > @@ -57,21 +57,24 @@ struct ipoib_ah *ipoib_create_ah(struct
> net_device *dev,
> > struct ib_pd *pd, struct ib_ah_attr *attr)
> > {
> > struct ipoib_ah *ah;
> > + struct ib_ah *vah;
> >
> > ah = kmalloc(sizeof *ah, GFP_KERNEL);
> > if (!ah)
> > - return NULL;
> > + return ERR_PTR(-ENOMEM);
> >
> > ah->dev = dev;
> > ah->last_send = 0;
> > kref_init(&ah->ref);
> >
> > - ah->ah = ib_create_ah(pd, attr);
> > - if (IS_ERR(ah->ah)) {
> > + vah = ib_create_ah(pd, attr);
> > + if (IS_ERR(vah)) {
> > kfree(ah);
> > - ah = NULL;
> > - } else
> > + ah = (struct ipoib_ah *)vah;
> > + } else {
> > + ah->ah = vah;
>
> This needs to be fixed. ah is freed, assign to an error code, then
> dereferenced.
This message and any attached documents contain information from QLogic
Corporation or its wholly-owned subsidiaries that may be confidential. If you
are not the intended recipient, you may not read, copy, distribute, or use this
information. If you have received this transmission in error, please notify the
sender immediately by reply e-mail and then delete this message.
N�����r��y����b�X��ǧv�^�){.n�+����{��ٚ�{ay�ʇڙ�,j��f���h���z��w���
���j:+v���w�j�m��������zZ+�����ݢj"��!�i