Re: [PATCH v2 bluetooth] 6lowpan: fix incorrect return values in lowpan_rcv

[Alexander Aring <alex.aring@gmail.com>]

On Mon, Sep 15, 2014 at 12:13:31PM +0200, Alexander Aring wrote:
...
> 
> >  			if (ret == 1) {
> > -				ret = process_data(skb, &hdr);
> > -				if (ret == NET_RX_DROP)
> > +				ret = process_data(&skb, &hdr);
> > +				if (ret < 0)
> >  					goto drop;
> this function returning an errno, you can't return the errno here. goto
> drop returns the ret variable. Maybe just make a return NET_RX_DROP at
> drop. We should never return a different variable.
Oops, I looked at the wong please I mean before drop, when all things
are fine.

> 
> Also this need to be drop_skb at several places in this code, we don't
> have a kfree_skb at process_data anymore. This leaks memory.
> 
> > +			} else {
> > +				return NET_RX_SUCCESS;
> >  			}
> >  			break;
> >  		case LOWPAN_DISPATCH_FRAGN:	/* next fragments headers */
> >  			ret = lowpan_frag_rcv(skb, LOWPAN_DISPATCH_FRAGN);
> 
> same here.
> >  			if (ret == 1) {
> > -				ret = process_data(skb, &hdr);
> > -				if (ret == NET_RX_DROP)
> > +				ret = process_data(&skb, &hdr);
> > +				if (ret < 0)
> >  					goto drop;
> 
> same here.
> 
> > +			} else {
> > +				return NET_RX_SUCCESS;
> >  			}
> >  			break;
> >  		default:
> > @@ -515,7 +514,16 @@ static int lowpan_rcv(struct sk_buff *skb, struct net_device *dev,
> >  		}
> >  	}
> >  

If all things are fine, there is somewhere a return ret;

We should change it to return NET_RX_SUCCESS.

- Alex