netdev - Re: Kernel memory leak in bnx2x driver with vxlan tunnel

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: <CAEh+42h1w=8nFi73afh8_B8_x3-kKSCBOkJZjE2v2QNfCAQ4oA@mail.gmail.com>
Date:	Tue, 19 Jan 2016 17:40:30 -0800
From:	Jesse Gross <jesse@...nel.org>
To:	Thomas Graf <tgraf@...g.ch>
Cc:	Eric Dumazet <eric.dumazet@...il.com>,
	John <john.phillips5@....com>,
	Linux Kernel Network Developers <netdev@...r.kernel.org>,
	Tom Herbert <tom@...bertland.com>, david.roth@....com,
	Pravin B Shelar <pshelar@...ira.com>
Subject: Re: Kernel memory leak in bnx2x driver with vxlan tunnel

On Tue, Jan 19, 2016 at 5:31 PM, Thomas Graf <tgraf@...g.ch> wrote:
> On 01/19/16 at 04:51pm, Jesse Gross wrote:
>> On Tue, Jan 19, 2016 at 4:17 PM, Eric Dumazet <eric.dumazet@...il.com> wrote:
>> > So what is the purpose of having a dst if we need to drop it ?
>> >
>> > Adding code in GRO would be fine if someone explains me the purpose of
>> > doing apparently useless work.
>> >
>> > (refcounting on dst is not exactly free)
>>
>> In the GRO case, the dst is only dropped on the packets which have
>> been merged and therefore need to be freed (the GRO_MERGED_FREE case).
>> It's not being thrown away for the overall frame, just metadata that
>> has been duplicated on each individual frame, similar to the metadata
>> in struct sk_buff itself. And while it is not used by the IP stack
>> there are other consumers (eBPF/OVS/etc.). This entire process is
>> controlled by the COLLECT_METADATA flag on tunnels, so there is no
>> cost in situations where it is not actually used.
>
> Right. There were thoughts around leveraging a per CPU scratch
> buffer without a refcount and turn it into a full reference when
> the packet gets enqueued somewhere but the need hasn't really come
> up yet.
>
> Jesse, is this what you have in mind:

Yes, that's what I was thinking. I also realized that we should
probably have a check in gro_list_prepare() to ensure that the dst of
the packets that we are about to merge are equal. And we should fix IP
early demux as Eric pointed out.