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
[email protected]
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
