From mboxrd@z Thu Jan 1 00:00:00 1970 From: Eric Dumazet Subject: Re: [PATCH v3] net: af_packet: don't call tpacket_destruct_skb() until the skb is sent out Date: Thu, 23 Sep 2010 14:29:30 +0200 Message-ID: <1285244970.2864.46.camel@edumazet-laptop> References: <1285236939-3239-1-git-send-email-xiaosuo@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: QUOTED-PRINTABLE Cc: "David S. Miller" , Oliver Hartkopp , "Michael S. Tsirkin" , netdev@vger.kernel.org To: Changli Gao Return-path: Received: from mail-fx0-f46.google.com ([209.85.161.46]:47546 "EHLO mail-fx0-f46.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753203Ab0IWM3f (ORCPT ); Thu, 23 Sep 2010 08:29:35 -0400 Received: by fxm12 with SMTP id 12so668380fxm.19 for ; Thu, 23 Sep 2010 05:29:34 -0700 (PDT) In-Reply-To: <1285236939-3239-1-git-send-email-xiaosuo@gmail.com> Sender: netdev-owner@vger.kernel.org List-ID: Le jeudi 23 septembre 2010 =C3=A0 18:15 +0800, Changli Gao a =C3=A9crit= : > Since skb->destructor() is used to account socket memory, and maybe c= alled > before the skb is sent out, a corrupt skb maybe sent out finally. >=20 > A new destructor is added into structure skb_shared_info(), and it wo= n't > be called until the last reference to the data of an skb is put. af_p= acket > uses this destructor instead. >=20 > Signed-off-by: Changli Gao > --- > v3: rename destructor to data_destructor, destructor_arg to data_dest= ructor_arg, > fix splice the skbs generated by AF_PACKET socket to the pipe. I dont understand this. Could you describe how splice(from af_packet to pipe) is possible with af_packet send path ? Also, on such risky patch, could you please avoid inserting cleanups ? I am referring to these bits : @@ -884,9 +883,8 @@ static int tpacket_fill_skb(struct packet_sock *po,= struct sk_buff *skb, to_write =3D tp_len; =20 if (sock->type =3D=3D SOCK_DGRAM) { - err =3D dev_hard_header(skb, dev, ntohs(proto), addr, - NULL, tp_len); - if (unlikely(err < 0)) + if (unlikely(dev_hard_header(skb, dev, ntohs(proto), ad= dr, + NULL, tp_len) < 0)) return -EINVAL; } else if (dev->hard_header_len) { /* net device doesn't like empty head */ @@ -897,8 +895,7 @@ static int tpacket_fill_skb(struct packet_sock *po, struct sk_buff *skb, } =20 skb_push(skb, dev->hard_header_len); - err =3D skb_store_bits(skb, 0, data, - dev->hard_header_len); + err =3D skb_store_bits(skb, 0, data, dev->hard_header_l= en); if (unlikely(err)) return err; =20 @@ -906,7 +903,6 @@ static int tpacket_fill_skb(struct packet_sock *po,= struct sk_buff *skb, to_write -=3D dev->hard_header_len; } =20 - err =3D -EFAULT; page =3D virt_to_page(data); offset =3D offset_in_page(data); len_max =3D PAGE_SIZE - offset;