netdev - Re: [PATCH net] net: add recursion limit to GRO

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:   Mon, 10 Oct 2016 07:03:56 -0700
From:   Eric Dumazet <eric.dumazet@...il.com>
To:     Sabrina Dubroca <sd@...asysnail.net>
Cc:     netdev@...r.kernel.org,
        Hannes Frederic Sowa <hannes@...essinduktion.org>,
        Jiri Benc <jbenc@...hat.com>
Subject: Re: [PATCH net] net: add recursion limit to GRO

On Mon, 2016-10-10 at 15:43 +0200, Sabrina Dubroca wrote:
> Currently, GRO can do unlimited recursion through the gro_receive
> handlers.  This was fixed for tunneling protocols by limiting tunnel GRO
> to one level with encap_mark, but both VLAN and TEB still have this
> problem.  Thus, the kernel is vulnerable to a stack overflow, if we
> receive a packet composed entirely of VLAN headers.
> 
> This patch adds a recursion counter to the GRO layer to prevent stack
> overflow.  When a gro_receive function hits the recursion limit, GRO is
> aborted for this skb and it is processed normally.
> 
> Thanks to Vladimír Beneš <vbenes@...hat.com> for the initial bug report.


Hi Sabrina

Have you considered using a per cpu counter ?

It might be cheaper than using a 4-bit field in skb.

Really this counter does not need to be stored in skb. GRO already uses
way too much space in skb->cb[]

Also please add appropriate unlikely() clauses, since most GRO traffic
is not trying to kill hosts ;)

Thanks.