netdev - Re: Bug in skb_gro_receive - possible bad page state problems?

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

Message-ID: <1493957685.7796.39.camel@edumazet-glaptop3.roam.corp.google.com>
Date:   Thu, 04 May 2017 21:14:45 -0700
From:   Eric Dumazet <eric.dumazet@...il.com>
To:     "Anand H. Krishnan" <anandhkrishnan@...il.com>
Cc:     netdev@...r.kernel.org
Subject: Re: Bug in skb_gro_receive - possible bad page state problems?

On Fri, 2017-05-05 at 08:57 +0530, Anand H. Krishnan wrote:
> Hello,
> 
> Is skb_gro_receive doing the right thing for cloned packets?
> 
> When we are merging fragments, we do not seem to be taking a reference
> to the underlying page. To me, it looks like it should work fine for non-cloned
> packets. However, for cloned packets, when the gro-ed packet is eventually
> freed (because the original skb was not cloned and hence reference was 1),
> the merged skb's frags also get freed (put_page-ed) without taking into account
> the other references that were held for the fragments (dataref).
> 
> We saw crashes because of this behavior. Our setup had a third party kernel
> forwarding module which uses GRO (napi_gro_receive). Doing iperf3 with small
> packets and doing tcpdump on the receiving tap interface results in the problem.
> With DEBUG_VM enabled, put page crashes. Without DEBUG_VM, bad page
> state results.

Yep, GRO must not be used with cloned skb.

This is why gro_cells_receive() has this check :

if (!gcells->cells || skb_cloned(skb) || netif_elide_gro(dev))
        return netif_rx(skb);

(But not the main napi_gro_receive() that is supposed to be used by
driver before any tap)