linux-kernel - Re: [PATCH v10] lib: checksum: Use aligned accesses for ip_fast_csum and csum_ipv6

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: <ZdzPgSCTntY7JD5i@shell.armlinux.org.uk>
Date: Mon, 26 Feb 2024 17:50:57 +0000
From: "Russell King (Oracle)" <linux@...linux.org.uk>
To: Guenter Roeck <linux@...ck-us.net>
Cc: Christophe Leroy <christophe.leroy@...roup.eu>,
	Charlie Jenkins <charlie@...osinc.com>,
	David Laight <David.Laight@...lab.com>,
	Palmer Dabbelt <palmer@...belt.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Helge Deller <deller@....de>,
	"James E.J. Bottomley" <James.Bottomley@...senpartnership.com>,
	Parisc List <linux-parisc@...r.kernel.org>,
	Arnd Bergmann <arnd@...db.de>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Palmer Dabbelt <palmer@...osinc.com>,
	Linux ARM <linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCH v10] lib: checksum: Use aligned accesses for ip_fast_csum
 and csum_ipv6_magic tests

On Mon, Feb 26, 2024 at 08:44:29AM -0800, Guenter Roeck wrote:
> On 2/26/24 03:34, Christophe Leroy wrote:
> > 
> > 
> > Le 23/02/2024 à 23:11, Charlie Jenkins a écrit :
> > > The test cases for ip_fast_csum and csum_ipv6_magic were not properly
> > > aligning the IP header, which were causing failures on architectures
> > > that do not support misaligned accesses like some ARM platforms. To
> > > solve this, align the data along (14 + NET_IP_ALIGN) bytes which is the
> > > standard alignment of an IP header and must be supported by the
> > > architecture.
> > 
> > I'm still wondering what we are really trying to fix here.
> > 
> > All other tests are explicitely testing that it works with any alignment.
> > 
> > Shouldn't ip_fast_csum() and csum_ipv6_magic() work for any alignment as
> > well ? I would expect it, I see no comment in arm code which explicits
> > that assumption around those functions.
> > 
> > Isn't the problem only the following line, because csum_offset is
> > unaligned ?
> > 
> > csum = *(__wsum *)(random_buf + i + csum_offset);
> > 
> > Otherwise, if there really is an alignment issue for the IPv6 source or
> > destination address, isn't it enough to perform a 32 bits alignment ?
> > 
> 
> It isn't just arm.
> 
> Question should be what alignments the functions are supposed to be able
> to handle, not what they are optimized for. If byte and/or half word alignments
> are expected to be supported, there is still architecture code which would
> have to be fixed. Unaligned accesses are known to fail on hppa64/parisc64
> and on sh4, for example. If unaligned accesses are expected to be handled,
> it would probably make sense to add a separate test case, though, to clarify
> that the test fails due to alignment issues, not due to input parameters.

It's network driver dependent. Most network drivers receive packets
to the offset defined by NET_IP_ALIGN (which is normally 2) which
has the effect of "mis-aligning" the ethernet header, but aligning
the IP header.

Whether drivers do that is up to drivers (and their capabilities).
Some network drivers can not do this kind of alignment, so there are
cases where the received packets aren't offset by two bytes, leading
to the IP header being aligned to an odd 16-bit word rather than an
even 16-bit word (and thus 32-bit aligned.)

Then you have the possibility of other headers between the ethernet
and IP header - not only things like VLANs, but also possibly DSA
headers (for switches) and how big those are.

There's a lot to be researched here!

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!