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: <20151011122118.GB2793@colbert.mtl.com>
Date:	Sun, 11 Oct 2015 15:21:18 +0300
From:	Ido Schimmel <idosch@...lanox.com>
To:	Nikolay Aleksandrov <razor@...ckwall.org>
CC:	<netdev@...r.kernel.org>, <roopa@...ulusnetworks.com>,
	<vyasevich@...il.com>, <stephen@...workplumber.org>,
	<bridge@...ts.linux-foundation.org>, <davem@...emloft.net>,
	"Nikolay Aleksandrov" <nikolay@...ulusnetworks.com>
Subject: Re: [PATCH net-next 4/5] bridge: vlan: fix possible null ptr derefs
 on port init and deinit

Wed, Sep 30, 2015 at 09:16:54PM IDT, razor@...ckwall.org wrote:
>From: Nikolay Aleksandrov <nikolay@...ulusnetworks.com>
>
>When a new port is being added we need to make vlgrp available after
>rhashtable has been initialized and when removing a port we need to
>flush the vlans and free the resources after we're sure noone can use
>the port, i.e. after it's removed from the port list and synchronize_rcu
>is executed.

Hi Nikolay,

Changing the order of port deinit breaks symmetry with the init
sequence. It also introduces a problem for switchdev drivers. Flushing
the VLANs clears HW VLAN filters and then, when port is unlinked from
bridge and CHANGEUPPER is received, port is configured to direct traffic
to CPU (as it's not offloaded anymore). Doing the reverse (like in this
patch) renders the port unusable.

Regarding the reason for this change, are you afraid that vlgrp will be
accessed in bridge's rx handler or xmit function after it's freed? If
so, maybe we can access it using RCU primitives? That way, both the rx
handler and xmit function (executed under RCU lock) will either have a
valid copy or not. Looking at previous iterations of this code, I see
that was the case with the 'net_port_vlans' struct.

I can start working on a fix if you agree with the proposed solution.

Thanks.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ