netdev - Re: [PATCH] vxlan: remove the redundant gro_cells

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:   Fri, 15 Mar 2019 22:55:52 +0800
From:   Zhiqiang Liu <liuzhiqiang26@...wei.com>
To:     Stefano Brivio <sbrivio@...hat.com>
CC:     <davem@...emloft.net>, <petrm@...lanox.com>, <idosch@...lanox.com>,
        <sd@...asysnail.net>, <mousuanming@...wei.com>,
        <netdev@...r.kernel.org>, Mingfangsen <mingfangsen@...wei.com>,
        "Zhoukang (A)" <zhoukang7@...wei.com>,
        "wangxiaogang (F)" <wangxiaogang3@...wei.com>
Subject: Re: [PATCH] vxlan: remove the redundant gro_cells_destroy() calling.

> On Fri, 15 Mar 2019 18:06:45 +0800
> Zhiqiang Liu <liuzhiqiang26@...wei.com> wrote:
> 
>> From: "Suanming.Mou" <mousuanming@...wei.com>
>>
>> With ad6c9986bcb6, GRO cells will be destroyed in vxlan_uninit.
> 
> Thanks for cleaning this up.
> 
> I think it would be nice if you could actually explain in the commit
> message why this makes the call in vxlan_destroy_tunnels() redundant.
> 
Thanks for your reply. Actually, the patch is a cleanup as you said.
In vxlan_destroy_tunnels func, unregister_netdevice_queue is called after gro_cells_destroy
func. However, in unregister_netdevice_queue func, the gro_cells_destroy func will also call
the gro_cells_destroy func as the following routine:
	unregister_netdevice_many -> rollback_registered_many -> ndo_uninit -> gro_cells_destroy

Fortunately, gro_cells_destroy func will check whether gcells->cells is NULL, so even more than
one call gro_cells_destroy would not cause the memory twice-free problem.

>> Fixes: ad6c9986bcb6 ("vxlan: Fix GRO cells race condition between receive and link delete")
> 
> I'm not sure a Fixes: tag is appropriate here (and also if this
> shouldn't be targeted for net-next) -- in the end, gro_cells_destroy()
> there would just return:
> 
>         if (!gcells->cells)
>                 return;
> 
>> Signed-off-by: Suanming.Mou <mousuanming@...wei.com>

As you said, this is just a cleanup. I will remove the Fixes tag in v2 patch.
I used the ./scripts/get_maintainer.pl to get the maintainers and mail-list, and the return is
given as follows,

[root@...alhost linux]# ./scripts/get_maintainer.pl 0001-vxlan-remove-the-redundant-gro_cells_destroy-calling.patch
"David S. Miller" <davem@...emloft.net> (odd fixer:NETWORKING DRIVERS,commit_signer:57/57=100%)
Petr Machata <petrm@...lanox.com> (commit_signer:30/57=53%,authored:27/57=47%,added_lines:649/1160=56%,removed_lines:265/494=54%)
Ido Schimmel <idosch@...lanox.com> (commit_signer:15/57=26%,removed_lines:30/494=6%)
Roopa Prabhu <roopa@...ulusnetworks.com> (commit_signer:11/57=19%,authored:9/57=16%,added_lines:364/1160=31%,removed_lines:156/494=32%)
Sabrina Dubroca <sd@...asysnail.net> (commit_signer:6/57=11%)
Stefano Brivio <sbrivio@...hat.com> (authored:5/57=9%,added_lines:63/1160=5%)
netdev@...r.kernel.org (open list:NETWORKING DRIVERS)
linux-kernel@...r.kernel.org (open list)

> 
> Either way,
> 
> Reviewed-by: Stefano Brivio <sbrivio@...hat.com>
>