netdev - Re: [PATCH net-next 3/3] net: allow to leave the buffer fragmented in skb_cow

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: <1383755989.4291.178.camel@edumazet-glaptop2.roam.corp.google.com>
Date:	Wed, 06 Nov 2013 08:39:49 -0800
From:	Eric Dumazet <eric.dumazet@...il.com>
To:	Herbert Xu <herbert@...dor.apana.org.au>
Cc:	Mathias Krause <mathias.krause@...unet.com>,
	"David S. Miller" <davem@...emloft.net>,
	Steffen Klassert <steffen.klassert@...unet.com>,
	Dmitry Tarnyagin <dmitry.tarnyagin@...kless.no>,
	netdev@...r.kernel.org
Subject: Re: [PATCH net-next 3/3] net: allow to leave the buffer fragmented
 in skb_cow_data()

On Wed, 2013-11-06 at 20:48 +0800, Herbert Xu wrote:
> On Wed, Nov 06, 2013 at 01:42:03PM +0100, Mathias Krause wrote:
> >
> > Well, skb_cow_data() will only copy, i.e. call __pskb_pull_tail(), in
> > case the skb is either cloned or fragmented. As you already said it
> > won't be cloned in your case. Does it contain fragments, i.e. is
> > skb_shinfo(skb)->nr_frags != 0? If not, we won't copy with the current
> > code either.
> 
> Whenever we say page it means nr_frags != 0.  So currently as
> long as we have pages in our skb we will copy.  With your patch
> we will no longer copy in the case where we have pages but the
> skb isn't cloned.  In fact that is the whole point of your patch.
> 
> > Can you please explain why this would be needed? I still don't get the
> > reasoning behind "pages are considered not writable at the moment even
> > if they are anonymous".
> 
> As I said you don't know where the page in the skb came from.  It
> may point to read-only memory or memory that's shared with another
> task that isn't expecting things to change underneath it.

Note that we might have now a per skb flag telling is all page frags are
owned.

This is SKBTX_SHARED_FRAG

If SKBTX_SHARED_FRAG is set on shared_info->tx_flags, then at least
one frag is not safe and we must copy all frags.


For example, this flag is set in TCP sendfile() path (vmsplice()...),
and zero copy paths in general (macvtap, tun)

I am not 100% sure, but this could be a hint.



--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html