netdev - Setting skb->csum in shared skb with csum-complete

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-next>] [day] [month] [year] [list]

Date:   Thu, 19 Jan 2017 17:29:09 -0800
From:   Tom Herbert <tom@...bertland.com>
To:     Herbert Xu <herbert@...dor.apana.org.au>,
        Linux Kernel Network Developers <netdev@...r.kernel.org>
Subject: Setting skb->csum in shared skb with csum-complete

Hi Herbert,

I am looking at "net: Fix skb csum races when peeking". The context is
that I'm seeing checksum faults after backporting mlx5 from driver.
The handling of the checksum faults in interesting in itself. One side
effect is that we end up recalculating and doing the dump twice when
GRO is enabled. Another is that we validate checksum complete in a
shared skbuf with a knowingly bad skb->csum value.

In the checksum fault case we recalculate the checksum and find it was
actually correct in the packet. If the skb is not shared then we set
the skb->csum to the correct value and also set skb->csum_complete_sw
and skb->csum_valid. But if skb is shared we don't do any of that.
This is kind of odd since __skb_checksum_complete_head then returns
zero for an skb with checksum_complete where the checksum complete
value is not correct. I'm not sure what the consequences of doing that
are (I don't think it's related to issues I'm seeing) but this doesn't
seem like a good thing. Maybe we should set skb->csum in this case
since we know it's already wrong and the fact that skbuf is shared
can't make it more wrong...

Tom