linux-kernel - Re: [PATCH v2] parisc: Strip upper 32 bit of sum in 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] [day] [month] [year] [list]

Message-ID: <00ea2941-0289-440b-8281-84d283d796b3@gmx.de>
Date: Tue, 27 Feb 2024 22:43:09 +0100
From: Helge Deller <deller@....de>
To: Guenter Roeck <linux@...ck-us.net>
Cc: "James E . J . Bottomley" <James.Bottomley@...senPartnership.com>,
 linux-parisc@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] parisc: Strip upper 32 bit of sum in csum_ipv6_magic
 for 64-bit builds

On 2/27/24 21:33, Guenter Roeck wrote:
> IPv6 checksum tests with unaligned addresses on 64-bit builds result
> in unexpected failures.
>
> Expected expected == csum_result, but
>      expected == 46591 (0xb5ff)
>      csum_result == 46381 (0xb52d)
> with alignment offset 1
>
> Oddly enough, the problem disappeared after adding test code into
> the beginning of csum_ipv6_magic().
>
> As it turns out, the 'sum' parameter of csum_ipv6_magic() is declared as
> __wsum, which is a 32-bit variable. However, it is treated as 64-bit
> variable in the 64-bit assembler code. Tests showed that the upper 32 bit
> of the register used to pass the variable are _not_ cleared when entering
> the function. This can result in checksum calculation errors.
>
> Clearing the upper 32 bit of 'sum' as first operation in the assembler
> code fixes the problem.
>
> Acked-by: Helge Deller <deller@....de>
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Signed-off-by: Guenter Roeck <linux@...ck-us.net>

applied.

Thanks!
Helge

> ---
> v2: Updated subject, clarified that the problem only affects 64-bit builds,
>      added Helge's Acked-by: tag.
>
>   arch/parisc/include/asm/checksum.h | 1 +
>   1 file changed, 1 insertion(+)
>
> diff --git a/arch/parisc/include/asm/checksum.h b/arch/parisc/include/asm/checksum.h
> index c949aa20fa16..2aceebcd695c 100644
> --- a/arch/parisc/include/asm/checksum.h
> +++ b/arch/parisc/include/asm/checksum.h
> @@ -126,6 +126,7 @@ static __inline__ __sum16 csum_ipv6_magic(const struct in6_addr *saddr,
>   	** Try to keep 4 registers with "live" values ahead of the ALU.
>   	*/
>
> +"	depdi		0, 31, 32, %0\n"/* clear upper half of incoming checksum */
>   "	ldd,ma		8(%1), %4\n"	/* get 1st saddr word */
>   "	ldd,ma		8(%2), %5\n"	/* get 1st daddr word */
>   "	add		%4, %0, %0\n"