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: <cdd09f7a-83b2-41ba-a32c-9886dd79c43e@roeck-us.net>
Date: Mon, 26 Feb 2024 15:48:43 -0800
From: Guenter Roeck <linux@...ck-us.net>
To: Charlie Jenkins <charlie@...osinc.com>,
 David Laight <David.Laight@...lab.com>
Cc: 'Russell King' <linux@...linux.org.uk>,
 Christophe Leroy <christophe.leroy@...roup.eu>,
 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 2/26/24 15:17, Charlie Jenkins wrote:
> On Mon, Feb 26, 2024 at 10:33:56PM +0000, David Laight wrote:
>> ...
>>> I think you misunderstand. "NET_IP_ALIGN offset is what the kernel
>>> defines to be supported" is a gross misinterpretation. It is not
>>> "defined to be supported" at all. It is the _preferred_ alignment
>>> nothing more, nothing less.
> 
> This distinction is arbitrary in practice, but I am open to being proven
> wrong if you have data to back up this statement. If the driver chooses
> to not follow this, then the driver might not work. ARM defines the
> NET_IP_ALIGN to be 2 to pad out the header to be on the supported
> alignment. If the driver chooses to pad with one byte instead of 2
> bytes, the driver may fail to work as the CPU may stall after the
> misaligned access.
> 
>>
>> I'm sure I've seen code that would realign IP headers to a 4 byte
>> boundary before processing them - but that might not have been in
>> Linux.
>>
>> I'm also sure there are cpu which will fault double length misaligned
>> memory transfers - which might be used to marginally speed up code.
>> Assuming more than 4 byte alignment for the IP header is likely
>> 'wishful thinking'.
>>
>> There is plenty of ethernet hardware that can only write frames
>> to even boundaries and plenty of cpu that fault misaligned accesses.
>> There are even cases of both on the same silicon die.
>>
>> You also pretty much never want a fault handler to fixup misaligned
>> ethernet frames (or really anything else for that matter).
>> It is always going to be better to check in the code itself.
>>
>> x86 has just made people 'sloppy' :-)
>>
>> 	David
>>
>> -
>> Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
>> Registration No: 1397386 (Wales)
>>
> 
> If somebody has a solution they deem to be better, I am happy to change
> this test case. Otherwise, I would appreciate a maintainer resolving
> this discussion and apply this fix.
> 
Agreed.

I do have a couple of patches which add explicit unaligned tests as well as
corner case tests (which are intended to trigger as many carry overflows
as possible). Once I get those working reliably, I'll be happy to submit
them as additional tests.

Thanks,
Guenter