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  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]
Date:	Mon, 10 Mar 2014 23:52:36 +0200
From:	"Michael S. Tsirkin" <mst@...hat.com>
To:	Herbert Xu <herbert@...dor.apana.org.au>
Cc:	linux-kernel@...r.kernel.org, jbenc@...hat.com,
	David Miller <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Daniel Borkmann <dborkman@...hat.com>,
	Simon Horman <horms@...ge.net.au>,
	Paul Durrant <Paul.Durrant@...rix.com>,
	Thomas Graf <tgraf@...g.ch>, Miklos Szeredi <mszeredi@...e.cz>,
	=?UTF-8?q?Peter=20Pan=28=E6=BD=98=E5=8D=AB=E5=B9=B3=29?= <panweiping3@...il.com>,
	netdev@...r.kernel.org
Subject: Re: [PATCH 5/5] skbuff: skb_segment: orphan frags before copying

On Tue, Mar 11, 2014 at 05:34:05AM +0800, Herbert Xu wrote:
> On Mon, Mar 10, 2014 at 06:29:36PM +0200, Michael S. Tsirkin wrote:
> > skb_segment copies frags around, so we need
> > to copy them carefully to avoid accessing
> > user memory after reporting completion to userspace
> > through a callback.
> > 
> > skb_segment doesn't normally happen on datapath:
> > TSO needs to be disabled - so disabling zero copy
> > in this case does not look like a big deal.
> > 
> > Signed-off-by: Michael S. Tsirkin <mst@...hat.com>
> 
> Why do we need this patch at all?

The cover letter has more detail:
	skb_segment ... moves frags
	between skbs without orphaning them.
	This causes userspace to assume it's safe to
	reuse the buffer, and receiver gets corrupted data.
	This further might leak information from the
	transmitter on the wire.

if still unclear, pls let me know.


> Contrary to what you said,
> this path is definitely performance-critical since the basis
> of being able to enable GSO by default is that it causes no
> regressions in non-TSO cases.
> 
> Cheers,

Well it's a single non taken branch.
If the branch is taken it's more expensive, but once we
take it once callback is invoked and zero copy is
disabled for future users, so it will be non taken from here on.


> -- 
> Email: Herbert Xu <herbert@...dor.apana.org.au>
> Home Page: http://gondor.apana.org.au/~herbert/
> PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
> --
> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists