netdev - Re: [PATCH v3 net 7/9] ipvlan: remove skb_share

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <db688bb4-bafa-8e9b-34aa-7f1d5a04e10f@gmail.com>
Date:   Tue, 11 Feb 2020 09:39:31 -0800
From:   Eric Dumazet <eric.dumazet@...il.com>
To:     "Jason A. Donenfeld" <Jason@...c4.com>, netdev@...r.kernel.org,
        davem@...emloft.net
Cc:     Mahesh Bandewar <maheshb@...gle.com>
Subject: Re: [PATCH v3 net 7/9] ipvlan: remove skb_share_check from xmit path



On 2/11/20 7:00 AM, Jason A. Donenfeld wrote:
> This is an impossible condition to reach; an skb in ndo_start_xmit won't
> be shared by definition.
> 

Yes, maybe, but can you elaborate in this changelog ?

AFAIK net/core/pktgen.c can definitely provide shared skbs.

     refcount_inc(&pkt_dev->skb->users);
     ret = dev_queue_xmit(pkt_dev->skb);

We might have to change pktgen to make sure we do not make skb shared
just because it was convenient.

Please do not give a link to some web page that might disappear in the future.

Having to follow an old thread to understand the reasoning is not appealing
for us having to fix bugs in the following years.

Thanks.

> Signed-off-by: Jason A. Donenfeld <Jason@...c4.com>
> Cc: Mahesh Bandewar <maheshb@...gle.com>
> Link: https://lore.kernel.org/netdev/CAHmME9pk8HEFRq_mBeatNbwXTx7UEfiQ_HG_+Lyz7E+80GmbSA@mail.gmail.com/
> ---
>  drivers/net/ipvlan/ipvlan_core.c | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/drivers/net/ipvlan/ipvlan_core.c b/drivers/net/ipvlan/ipvlan_core.c
> index 30cd0c4f0be0..da40723065f2 100644
> --- a/drivers/net/ipvlan/ipvlan_core.c
> +++ b/drivers/net/ipvlan/ipvlan_core.c
> @@ -605,9 +605,6 @@ static int ipvlan_xmit_mode_l2(struct sk_buff *skb, struct net_device *dev)
>  				return ipvlan_rcv_frame(addr, &skb, true);
>  			}
>  		}
> -		skb = skb_share_check(skb, GFP_ATOMIC);
> -		if (!skb)
> -			return NET_XMIT_DROP;
>  
>  		/* Packet definitely does not belong to any of the
>  		 * virtual devices, but the dest is local. So forward
>