Return-Path: Date: Tue, 16 Sep 2014 13:36:16 +0200 From: Alexander Aring To: Martin Townsend Cc: linux-zigbee-devel@lists.sourceforge.net, linux-bluetooth@vger.kernel.org, linux-wpan@vger.kernel.org, marcel@holtmann.org, Martin Townsend Subject: Re: [PATCH v4 bluetooth] 6lowpan: fix incorrect return values in lowpan_rcv Message-ID: <20140916113614.GC4969@omega> References: <1410865319-16310-1-git-send-email-martin.townsend@xsilon.com> <1410865319-16310-2-git-send-email-martin.townsend@xsilon.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 In-Reply-To: <1410865319-16310-2-git-send-email-martin.townsend@xsilon.com> List-ID: On Tue, Sep 16, 2014 at 12:01:59PM +0100, Martin Townsend wrote: > Currently there are a number of error paths in the lowpan_rcv function that > free the skb before returning, the patch simplifies the receive path by > ensuring that the skb is only freed from this function. > > Passing the skb from 6lowpan up to the higher layers is not a > function of IPHC. By moving it out of IPHC we also remove the > need to support error code returns with NET_RX codes. > It also makes the lowpan_rcv function more extendable as we > can support more compression schemes. > > With the above 2 lowpan_rcv is refacored so eliminate incorrect return values. > > Signed-off-by: Martin Townsend > --- > include/net/6lowpan.h | 9 +++-- > net/6lowpan/iphc.c | 88 +++++++++++++++++-------------------------- > net/bluetooth/6lowpan.c | 38 ++++++++++--------- > net/ieee802154/6lowpan_rtnl.c | 61 ++++++++++++++++-------------- > 4 files changed, 94 insertions(+), 102 deletions(-) > > diff --git a/include/net/6lowpan.h b/include/net/6lowpan.h > index d184df1..c28fadb 100644 > --- a/include/net/6lowpan.h > +++ b/include/net/6lowpan.h ... > -static int process_data(struct sk_buff *skb, struct net_device *netdev, > - struct l2cap_chan *chan) > +static struct sk_buff * > +process_data(struct sk_buff *skb, struct net_device *netdev, > + struct l2cap_chan *chan) > { > const u8 *saddr, *daddr; > u8 iphc0, iphc1; > @@ -230,36 +231,31 @@ static int process_data(struct sk_buff *skb, struct net_device *netdev, > peer = peer_lookup_chan(dev, chan); > read_unlock_irqrestore(&devices_lock, flags); > if (!peer) > - goto drop; > + return ERR_PTR(-EINVAL); > > saddr = peer->eui64_addr; > daddr = dev->netdev->dev_addr; > > /* at least two bytes will be used for the encoding */ > if (skb->len < 2) > - goto drop; > + return ERR_PTR(-EINVAL); > > if (lowpan_fetch_skb_u8(skb, &iphc0)) > - goto drop; > + return ERR_PTR(-EINVAL); > > if (lowpan_fetch_skb_u8(skb, &iphc1)) > - goto drop; > + return ERR_PTR(-EINVAL); > > return lowpan_process_data(skb, netdev, > saddr, IEEE802154_ADDR_LONG, EUI64_ADDR_LEN, > daddr, IEEE802154_ADDR_LONG, EUI64_ADDR_LEN, > - iphc0, iphc1, give_skb_to_upper); > - > -drop: > - kfree_skb(skb); > - return -EINVAL; > + iphc0, iphc1); > } > > static int recv_pkt(struct sk_buff *skb, struct net_device *dev, > struct l2cap_chan *chan) > { > struct sk_buff *local_skb; > - int ret; > > if (!netif_running(dev)) > goto drop; > @@ -280,9 +276,6 @@ static int recv_pkt(struct sk_buff *skb, struct net_device *dev, > local_skb->protocol = htons(ETH_P_IPV6); > local_skb->pkt_type = PACKET_HOST; > > - skb_reset_network_header(local_skb); > - skb_set_transport_header(local_skb, sizeof(struct ipv6hdr)); > - > if (give_skb_to_upper(local_skb, dev) != NET_RX_SUCCESS) { > kfree_skb(local_skb); > goto drop; > @@ -300,9 +293,20 @@ static int recv_pkt(struct sk_buff *skb, struct net_device *dev, > if (!local_skb) > goto drop; > > - ret = process_data(local_skb, dev, chan); > - if (ret != NET_RX_SUCCESS) > + skb = process_data(local_skb, dev, chan); > + if (IS_ERR(skb)) { > + kfree_skb(local_skb); > goto drop; > + } > + > + local_skb->protocol = htons(ETH_P_IPV6); > + local_skb->pkt_type = PACKET_HOST; this is the wrong skb here, the new one is skb. I don't know maybe there is some optimization to call skb = process_data(skb, ...); > + > + if (give_skb_to_upper(local_skb, dev) also here local_skb should be skb, or? - Alex