netdev - Re: [PATCH net 1/4] net: fix double-free on fraglist GSO skbs

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: <20210511093953.GT40979@gauss3.secunet.de>
Date:   Tue, 11 May 2021 11:39:53 +0200
From:   Steffen Klassert <steffen.klassert@...unet.com>
To:     Paolo Abeni <pabeni@...hat.com>
CC:     Willem de Bruijn <willemdebruijn.kernel@...il.com>,
        "David S. Miller" <davem@...emloft.net>,
        Network Development <netdev@...r.kernel.org>,
        "Willem de Bruijn" <willemb@...gle.com>,
        Miaohe Lin <linmiaohe@...wei.com>,
        "Jakub Kicinski" <kuba@...nel.org>
Subject: Re: [PATCH net 1/4] net: fix double-free on fraglist GSO skbs

On Mon, May 10, 2021 at 05:37:58PM +0200, Paolo Abeni wrote:
> 
> It's taking [much] more than expected, as it turned out that thare are
> still a number of case where the tx csum is uncorrect.
> 
> If the traffic comes from a veth we don't have a valid th->csum value
> at GRO time, setting ip_summed to CHECKSUM_UNNECESSARY - as the current
> code does - looks wrong.
> @Steffen: I see in the original discussion about GRO_FRAGLIST
> introduction that you wanted the GRO packets to be CHECKSUM_UNNECESSARY
> to avoid csum modification in fwd path. I guess that choice was mostily
> due performance reasons, to avoid touching the aggregated pkts header
> at gso_segment_list time, but it looks like it's quite bug prone. If
> so, I'm unsure the performance gain is worty.

Yes, that was for performance reasons. We don't mangle the packets
with fraglist GRO, so the checksum should be still correct when
doing GSO.

> I propose to switch to
> CHECKSUM_PARTIAL. Would you be ok with that?

If there are cases where CHECKSUM_UNNECESSARY is problematic,
then yes, let's switch to CHECKSUM_PARTIAL.

Thanks for doing this Paolo!