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]
Date:	Tue, 28 Oct 2014 13:01:57 +0000
From:	David Laight <David.Laight@...LAB.COM>
To:	'Lothar Waßmann' <LW@...O-electronics.de>
CC:	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"David S. Miller" <davem@...emloft.net>,
	Russell King <rmk+kernel@....linux.org.uk>,
	Frank Li <Frank.Li@...escale.com>,
	Fabio Estevam <fabio.estevam@...escale.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>
Subject: RE: [PATCHv2 6/6] net: fec: fix regression on i.MX28 introduced by
 rx_copybreak support

From: Lothar Waßmann
> David Laight wrote:
> > From: Lothar Waßmann
> > > commit 1b7bde6d659d ("net: fec: implement rx_copybreak to improve rx performance")
> > > introduced a regression for i.MX28. The swap_buffer() function doing
> > > the endian conversion of the received data on i.MX28 may access memory
> > > beyond the actual packet size in the DMA buffer. fec_enet_copybreak()
> > > does not copy those bytes, so that the last bytes of a packet may be
> > > filled with invalid data after swapping.
> > > This will likely lead to checksum errors on received packets.
> > > E.g. when trying to mount an NFS rootfs:
> > > UDP: bad checksum. From 192.168.1.225:111 to 192.168.100.73:44662 ulen 36
> > >
> > > Do the byte swapping and copying to the new skb in one go if
> > > necessary.
> > >
> > > Signed-off-by: Lothar Wamann <LW@...O-electronics.de>
> > > ---
> > >  drivers/net/ethernet/freescale/fec_main.c |   25 +++++++++++++++++++++----
> > >  1 file changed, 21 insertions(+), 4 deletions(-)
> > >
> > > @@ -1455,7 +1472,7 @@ fec_enet_rx_queue(struct net_device *ndev, int budget, u16 queue_id)
> > >  		prefetch(skb->data - NET_IP_ALIGN);
> > >  		skb_put(skb, pkt_len - 4);
> > >  		data = skb->data;
> > > -		if (fep->quirks & FEC_QUIRK_SWAP_FRAME)
> > > +		if (!is_copybreak && need_swap)
> > >  			swap_buffer(data, pkt_len);
> >
> > It has to be better to set the 'copybreak' limit to be larger than the
> > maximum frame size and so always go through the 'copybreak' paths.
> >
> Since the copybreak support is all about performance optimistation, we
> should IMO buy the additional advantage for i.MX28 by not having to
> access the buffer twice (once for copying and once again for swapping).

You definitely want to do the byteswap at the same time as the copy.

The point I'm trying to make that if you need to do the byteswap you
probably might as well copy the data to an skb of the correct size at
the same time.
Certainly I'd expect the 'break even' length will be much higher.

	David


Powered by blists - more mailing lists