From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ido Schimmel Subject: [PATCH net 4/4] mlxsw: spectrum_switchdev: Fix VLAN device deletion via ioctl Date: Thu, 6 Dec 2018 17:44:53 +0000 Message-ID: <20181206174427.4952-5-idosch@mellanox.com> References: <20181206174427.4952-1-idosch@mellanox.com> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Cc: "davem@davemloft.net" , Jiri Pirko , Nir Dotan , Petr Machata , Ido Schimmel To: "netdev@vger.kernel.org" Return-path: Received: from mail-eopbgr40059.outbound.protection.outlook.com ([40.107.4.59]:16160 "EHLO EUR03-DB5-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1726079AbeLFRp1 (ORCPT ); Thu, 6 Dec 2018 12:45:27 -0500 In-Reply-To: <20181206174427.4952-1-idosch@mellanox.com> Content-Language: en-US Sender: netdev-owner@vger.kernel.org List-ID: When deleting a VLAN device using an ioctl the netdev is unregistered before the VLAN filter is updated via ndo_vlan_rx_kill_vid(). It can lead to a use-after-free in mlxsw in case the VLAN device is deleted while being enslaved to a bridge. The reason for the above is that when mlxsw receives the CHANGEUPPER event, it wrongly assumes that the VLAN device is no longer its upper and thus destroys the internal representation of the bridge port despite the reference count being non-zero. Fix this by checking if the VLAN device is our upper using its real device. In net-next I'm going to remove this trick and instead make mlxsw completely agnostic to the order of the events. Fixes: c57529e1d5d8 ("mlxsw: spectrum: Replace vPorts with Port-VLAN") Signed-off-by: Ido Schimmel Reviewed-by: Petr Machata --- .../net/ethernet/mellanox/mlxsw/spectrum_switchdev.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c b/dri= vers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c index 7f2091c2648e..50080c60a279 100644 --- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c +++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c @@ -296,7 +296,13 @@ static bool mlxsw_sp_bridge_port_should_destroy(const struct mlxsw_sp_bridge_port * bridge_port) { - struct mlxsw_sp *mlxsw_sp =3D mlxsw_sp_lower_get(bridge_port->dev); + struct net_device *dev =3D bridge_port->dev; + struct mlxsw_sp *mlxsw_sp; + + if (is_vlan_dev(dev)) + mlxsw_sp =3D mlxsw_sp_lower_get(vlan_dev_real_dev(dev)); + else + mlxsw_sp =3D mlxsw_sp_lower_get(dev); =20 /* In case ports were pulled from out of a bridged LAG, then * it's possible the reference count isn't zero, yet the bridge @@ -2109,7 +2115,7 @@ mlxsw_sp_bridge_8021d_port_leave(struct mlxsw_sp_brid= ge_device *bridge_device, =20 vid =3D is_vlan_dev(dev) ? vlan_dev_vlan_id(dev) : 1; mlxsw_sp_port_vlan =3D mlxsw_sp_port_vlan_find_by_vid(mlxsw_sp_port, vid)= ; - if (WARN_ON(!mlxsw_sp_port_vlan)) + if (!mlxsw_sp_port_vlan) return; =20 mlxsw_sp_port_vlan_bridge_leave(mlxsw_sp_port_vlan); --=20 2.19.1