On Mon, Aug 03, 2015 at 08:23:45AM +0200, Alexander Aring wrote: > This patch moves some trivial checks at first before calling > skb_share_check which could do some memcpy if the buffer is shared. > > Signed-off-by: Alexander Aring <alex.aring@xxxxxxxxx> > --- > net/ieee802154/6lowpan/rx.c | 10 ++++------ > 1 file changed, 4 insertions(+), 6 deletions(-) > > diff --git a/net/ieee802154/6lowpan/rx.c b/net/ieee802154/6lowpan/rx.c > index 7b6b038..11a5629 100644 > --- a/net/ieee802154/6lowpan/rx.c > +++ b/net/ieee802154/6lowpan/rx.c > @@ -62,16 +62,14 @@ static int lowpan_rcv(struct sk_buff *skb, struct net_device *wdev, > struct ieee802154_hdr hdr; > int ret; > > + if (skb->pkt_type == PACKET_OTHERHOST || > + wdev->type != ARPHRD_IEEE802154) > + goto drop; > + > skb = skb_share_check(skb, GFP_ATOMIC); > if (!skb) > goto drop; mhhh, nothing according to the patch but I think "skb_share_check" is wrong here and this depends on the dispatch value. Why it's wrong? When we do iphc_decompress we manipulate the buffer but "skb_share_check" unshare the "skb structure" only. That's the different between skb_clone and skb_copy. skb_clone doesn't "copy" the buffer, the buffer is still shared afterwards when it's shared. Sometimes like ipv6 dispatch we only run skb_pull, then we need "skb_share_check" only, because skb_pull doesn't manipulate the buffer, it's doing only some pointer moving inside "skb structure" so skb_clone if it's shared is enough, skb_clone is too much. I would remove this from the lowpan_rcv and we need to care that this function does read_only on "skb buffer" and "skb structure", then we don't need to call anything. To do "skb_share_check" (skb_clone if shared) or "skb_unshare" (skb_copy if shared), should be handled by the dispatch handlers lowpan_rx_h_ipv6, lowpan_rx_h_iphc, lowpan_rx_h_.... - Alex -- To unsubscribe from this list: send the line "unsubscribe linux-wpan" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
