netdev - Re: [PATCH] net: Convert skb->csum_(start|offset) integrity BUG_ON() to WARN

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: <20130213.193759.1094469499685059296.davem@davemloft.net>
Date:	Wed, 13 Feb 2013 19:37:59 -0500 (EST)
From:	David Miller <davem@...emloft.net>
To:	tgraf@...g.ch
Cc:	netdev@...r.kernel.org
Subject: Re: [PATCH] net: Convert skb->csum_(start|offset) integrity
 BUG_ON() to WARN_ON() & drop

From: Thomas Graf <tgraf@...g.ch>
Date: Wed, 13 Feb 2013 23:48:43 +0000

> On 02/13/13 at 11:40pm, Thomas Graf wrote:
>> They have been hit with IPoIB which uses a 64K MTU. If a TCP
>> retransmission gets partially ACKed and collapsed multiple times
>> it is possible for the headroom to grow beyond 64K which will
>> overflow the 16bit skb->csum_start.
> 
> On the subject of fixing this, I considered:
> 
>  a) Reallocate the headroom in tcp_trim_head() if it would
>     overflow. I disregarded this idea because replacing the
>     old skb with the new skb on the write queue for such a
>     rare situation seems overkill.
> 
>  b) No longer collapse if the new skb would result in a
>     a headroom + data that exceeds 64K. This seems to be the
>     most trivial fix.
> 
>  c) Increase size of csum_start or store checksum_start_offset
>     differently.
> 
> Other ideas?

"b" is a good idea.

Let's not paper over this, this BUG_ON() is really a BUG_ON()
meaning "FIX ME NOW" :-)
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html