netdev - Re: [PATCH net] powerpc: use big endian to hash len and proto 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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20180908183327.Horde.z6jJloy6XgHTgH9gNgohmQ1@messagerie.si.c-s.fr>
Date:   Sat, 08 Sep 2018 18:33:27 +0200
From:   LEROY Christophe <christophe.leroy@....fr>
To:     Xin Long <lucien.xin@...il.com>
Cc:     Roopa Prabhu <roopa@...ulusnetworks.com>,
        Michael Ellerman <mpe@...erman.id.au>,
        linuxppc-dev@...ts.ozlabs.org, network dev <netdev@...r.kernel.org>
Subject: Re: [PATCH net] powerpc: use big endian to hash len and proto in
 csum_ipv6_magic

Xin Long <lucien.xin@...il.com> a écrit :

> The function csum_ipv6_magic doesn't convert len and proto to big
> endian before doing ipv6 csum hash, which is not consistent with
> RFC and other arches.
>
> Jianlin found it when ICMPv6 packets from other hosts were dropped
> in the powerpc64 system.
>
> This patch is to fix it by using instruction 'lwbrx' to do this
> conversion in powerpc32/64 csum_ipv6_magic.
>
> Fixes: e9c4943a107b ("powerpc: Implement csum_ipv6_magic in assembly")
> Reported-by: Jianlin Shi <jishi@...hat.com>
> Signed-off-by: Xin Long <lucien.xin@...il.com>
> ---
>  arch/powerpc/lib/checksum_32.S | 4 ++++
>  arch/powerpc/lib/checksum_64.S | 4 ++++
>  2 files changed, 8 insertions(+)
>
> diff --git a/arch/powerpc/lib/checksum_32.S b/arch/powerpc/lib/checksum_32.S
> index aa22406..7d3446e 100644
> --- a/arch/powerpc/lib/checksum_32.S
> +++ b/arch/powerpc/lib/checksum_32.S
> @@ -325,6 +325,10 @@ _GLOBAL(csum_ipv6_magic)
>  	adde	r0, r0, r9
>  	lwz	r11, 12(r4)
>  	adde	r0, r0, r10
> +	STWX_BE	r5, 0, r1
> +	lwz	r5, 0(r1)
> +	STWX_BE	r6, 0, r1
> +	lwz	r6, 0(r1)

PPC32 doesn't support little endian, so nothing to do here.

>  	add	r5, r5, r6	/* assumption: len + proto doesn't carry */
>  	adde	r0, r0, r11
>  	adde	r0, r0, r5
> diff --git a/arch/powerpc/lib/checksum_64.S b/arch/powerpc/lib/checksum_64.S
> index 886ed94..302e732 100644
> --- a/arch/powerpc/lib/checksum_64.S
> +++ b/arch/powerpc/lib/checksum_64.S
> @@ -439,6 +439,10 @@ EXPORT_SYMBOL(csum_partial_copy_generic)
>  _GLOBAL(csum_ipv6_magic)
>  	ld	r8, 0(r3)
>  	ld	r9, 8(r3)
> +	STWX_BE	r5, 0, r1
> +	lwz	r5, 0(r1)
> +	STWX_BE	r6, 0, r1
> +	lwz	r6, 0(r1)
>  	add	r5, r5, r6

This is overkill. For LE it should be enough to rotate r5 by 8 bits  
after the sum. Best place to do it would be after ld r11 I think.

Christophe

>  	addc	r0, r8, r9
>  	ld	r10, 0(r4)
> --
> 2.1.0