netdev - Re: [PATCH net-next] udp: Complement partial checksum for GSO packet

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: <CAF=yD-+L6ZJq9E0qasPXnA0p3SB=hag+RykO3V707yrrU9-8dA@mail.gmail.com>
Date:   Tue, 1 May 2018 12:06:27 +0200
From:   Willem de Bruijn <willemdebruijn.kernel@...il.com>
To:     Sean Tranchetti <stranche@...eaurora.org>
Cc:     Willem de Bruijn <willemb@...gle.com>,
        David Miller <davem@...emloft.net>,
        Network Development <netdev@...r.kernel.org>,
        Subash Abhinov Kasiviswanathan <subashab@...eaurora.org>
Subject: Re: [PATCH net-next] udp: Complement partial checksum for GSO packet

On Tue, May 1, 2018 at 2:01 AM, Sean Tranchetti <stranche@...eaurora.org> wrote:
> Using the udp_v4_check() function to calculate the pseudo header
> for the newly segmented UDP packets results in assigning the complement
> of the value to the UDP header checksum field.
>
> Always undo the complement the partial checksum value in order to
> match the case where GSO is not used on the UDP transmit path.
>
> Fixes: ee80d1ebe5ba ("udp: add udp gso")
> Signed-off-by: Sean Tranchetti <stranche@...eaurora.org>
> Signed-off-by: Subash Abhinov Kasiviswanathan <subashab@...eaurora.org>
> ---
>  net/ipv4/udp_offload.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/net/ipv4/udp_offload.c b/net/ipv4/udp_offload.c
> index f78fb36..0062570 100644
> --- a/net/ipv4/udp_offload.c
> +++ b/net/ipv4/udp_offload.c
> @@ -223,6 +223,7 @@ struct sk_buff *__udp_gso_segment(struct sk_buff *gso_skb,
>                         csum_replace2(&uh->check, htons(mss),
>                                       htons(seg->len - hdrlen - sizeof(*uh)));
>
> +               uh->check = ~uh->check;
>                 seg->destructor = sock_wfree;
>                 seg->sk = sk;
>                 sum_truesize += seg->truesize;

Thanks, this looks plausible. I may have been mistaken by evaluation
showing packets arriving at the peer with the correct checksum before.

On retesting, packets arrive with correct checksum in both cases, and
even when setting purposely bad uh->check values.

Perhaps the two platforms I tested on (bnx2x, mlx4) ignore the pseudo
header and recompute from scratch if CHECKSUM_PARTIAL is set for
udp. Still having a look..