On Tue, Jun 20, 2017 at 10:56:43PM +0300, Dan Carpenter wrote:
> On Tue, Jun 20, 2017 at 05:11:51PM +0200, Christian Gromm wrote:
> > @@ -411,21 +428,31 @@ static int aim_rx_data(struct mbo *mbo)
> >     struct sk_buff *skb;
> >     struct net_device *dev;
> >     unsigned int skb_len;
> > +   int ret = 0;
> >  
> > -   nd = get_net_dev_context(mbo->ifp);
> > -   if (!nd || nd->rx.ch_id != mbo->hdm_channel_id)
> > +   nd = get_net_dev_hold(mbo->ifp);
> > +   if (!nd)
> >             return -EIO;
> >  
> > +   if (nd->rx.ch_id != mbo->hdm_channel_id) {
> > +           ret = -EIO;
> > +           goto put_nd;
> > +   }
> > +
> >     dev = nd->dev;
> >  
> >     if (nd->is_mamac) {
> > -           if (!PMS_IS_MAMAC(buf, len))
> > -                   return -EIO;
> > +           if (!PMS_IS_MAMAC(buf, len)) {
> > +                   ret = -EIO;
> > +                   goto put_nd;
> > +           }
> >  
> >             skb = dev_alloc_skb(len - MDP_HDR_LEN + 2 * ETH_ALEN + 2);
> >     } else {
> > -           if (!PMS_IS_MEP(buf, len))
> > -                   return -EIO;
> > +           if (!PMS_IS_MEP(buf, len)) {
> > +                   ret = -EIO;
> > +                   goto put_nd;
> > +           }
> >  
> >             skb = dev_alloc_skb(len - MEP_HDR_LEN);
> >     }
> > @@ -468,7 +495,10 @@ static int aim_rx_data(struct mbo *mbo)
> >  
> >  out:
> >     most_put_mbo(mbo);
> > -   return 0;
> > +
> > +put_nd:
> > +   dev_put(nd->dev);
> > +   return ret;
> >  }
> >  
> 
> Can we actually hit any of the goto put_nd paths?  I know you didn't
> introduce this but it feels like we should be calling most_put_mbo() on
> basically all those paths or we'r leaking.  I'm not really familiar with

The most_put_mbo must be called only if the aim_rx_data() returns zero
(packet is processed).  Each put_nd path returns an error.

> the code, and those also slightly feel like sanity checks which we don't
> actually think can happen...  ?
> 

The first goto put_nd is really the sanity check and may be omitted.

The checks PMS_IS_MAMAC and PMS_IS_MEP are needed to pass the MDP
packets (not MAMAC/MEP) processed by the other application modules.

-- 
regards,
andy

_______________________________________________
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Reply via email to