netdev - Re: [PATCH net] net/mlx5e: fix csum adjustments caused by RXFCS

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:   Wed, 31 Oct 2018 11:27:46 +0000
From:   Eran Ben Elisha <eranbe@...lanox.com>
To:     Eric Dumazet <edumazet@...gle.com>,
        "David S . Miller" <davem@...emloft.net>
CC:     netdev <netdev@...r.kernel.org>,
        Eric Dumazet <eric.dumazet@...il.com>,
        Saeed Mahameed <saeedm@...lanox.com>,
        Dimitris Michailidis <dmichail@...gle.com>,
        Cong Wang <xiyou.wangcong@...il.com>,
        Paweł Staszewski <pstaszewski@...are.pl>,
        Maria Pasechnik <mariap@...lanox.com>
Subject: Re: [PATCH net] net/mlx5e: fix csum adjustments caused by RXFCS



On 10/30/2018 9:57 AM, Eric Dumazet wrote:
> As shown by Dmitris, we need to use csum_block_add() instead of csum_add()
> when adding the FCS contribution to skb csum.
> 
> Before 4.18 (more exactly commit 88078d98d1bb "net: pskb_trim_rcsum()
> and CHECKSUM_COMPLETE are friends"), the whole skb csum was thrown away,
> so RXFCS changes were ignored.
> 
> Then before commit d55bef5059dd ("net: fix pskb_trim_rcsum_slow() with
> odd trim offset") both mlx5 and pskb_trim_rcsum_slow() bugs were canceling
> each other.
> 
> Now we fixed pskb_trim_rcsum_slow() we need to fix mlx5.
> 
> Note that this patch also rewrites mlx5e_get_fcs() to :
> 
> - Use skb_header_pointer() instead of reinventing it.
> - Use __get_unaligned_cpu32() to avoid possible non aligned accesses
>    as Dmitris pointed out.
> 
> Fixes: 902a545904c7 ("net/mlx5e: When RXFCS is set, add FCS data into checksum calculation")
> Reported-by: Paweł Staszewski <pstaszewski@...are.pl>
> Signed-off-by: Eric Dumazet <edumazet@...gle.com>
> Cc: Eran Ben Elisha <eranbe@...lanox.com>
> Cc: Saeed Mahameed <saeedm@...lanox.com>
> Cc: Dimitris Michailidis <dmichail@...gle.com>
> Cc: Cong Wang <xiyou.wangcong@...il.com>
> Cc: Paweł Staszewski <pstaszewski@...are.pl>
> ---
>   .../net/ethernet/mellanox/mlx5/core/en_rx.c   | 45 ++++---------------
>   1 file changed, 9 insertions(+), 36 deletions(-)

Thanks for the modification!
We run a direct test we had for this scenario and it passed.

Reviewed-by: Eran Ben Elisha <eranbe@...lanox.com>
Tested-By: Maria Pasechnik <mariap@...lanox.com>