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 code, and those also slightly feel like sanity checks which we don't
actually think can happen...  ?

What do you think?

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

Reply via email to