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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 10 Oct 2016 18:20:35 +0200
From:   Sabrina Dubroca <sd@...asysnail.net>
To:     Eric Dumazet <eric.dumazet@...il.com>
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

Hi Eric,

2016-10-10, 07:03:56 -0700, Eric Dumazet wrote:
> 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.

I thought about it, but this looked a bit simpler. I can try some
benchmarking tomorrow.


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

For net-next I'm working on turning GRO into a loop, which would
eliminate these few bits.


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

Right. I'll send a v2 with this later.


Thanks,

-- 
Sabrina