netdev - [PATCH net-next 01/12] mlxsw: spectrum_mr: Publish multicast route after writing it to the device

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: <20200221175415.390884-2-idosch@idosch.org>
Date:   Fri, 21 Feb 2020 19:54:04 +0200
From:   Ido Schimmel <idosch@...sch.org>
To:     netdev@...r.kernel.org
Cc:     davem@...emloft.net, jiri@...lanox.com, mlxsw@...lanox.com,
        Ido Schimmel <idosch@...lanox.com>
Subject: [PATCH net-next 01/12] mlxsw: spectrum_mr: Publish multicast route after writing it to the device

From: Ido Schimmel <idosch@...lanox.com>

The driver periodically traverses the linked list of multicast routes
and updates the kernel about packets and bytes statistics from each
multicast route. These statistics are read from a counter associated
with the route when it is written to the device.

Currently, multicast routes are published via this linked list before
they are associated with a counter. Despite that, it is not possible for
the driver to access an invalid counter because the delayed work that
reads the statistics and multicast route addition / deletion are
mutually exclusive using RTNL.

In order to be able to remove RTNL, the list needs to be protected by a
dedicated lock, but any route published via the list must have an
associated counter, otherwise the driver will access an invalid counter.

Solve this by re-ordering the operations during multicast route addition
so that the route is only added to the linked list after it was written
to the device. Similarly, during deletion the route is first removed
from the linked list before its deletion from the device.

Signed-off-by: Ido Schimmel <idosch@...lanox.com>
Acked-by: Jiri Pirko <jiri@...lanox.com>
---
 .../net/ethernet/mellanox/mlxsw/spectrum_mr.c   | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_mr.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_mr.c
index 423eedebcd22..e40437d5aa76 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_mr.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_mr.c
@@ -371,10 +371,10 @@ static void __mlxsw_sp_mr_route_del(struct mlxsw_sp_mr_table *mr_table,
 				    struct mlxsw_sp_mr_route *mr_route)
 {
 	mlxsw_sp_mr_mfc_offload_set(mr_route, false);
-	mlxsw_sp_mr_route_erase(mr_table, mr_route);
 	rhashtable_remove_fast(&mr_table->route_ht, &mr_route->ht_node,
 			       mlxsw_sp_mr_route_ht_params);
 	list_del(&mr_route->node);
+	mlxsw_sp_mr_route_erase(mr_table, mr_route);
 	mlxsw_sp_mr_route_destroy(mr_table, mr_route);
 }
 
@@ -415,6 +415,11 @@ int mlxsw_sp_mr_route_add(struct mlxsw_sp_mr_table *mr_table,
 		goto err_duplicate_route;
 	}
 
+	/* Write the route to the hardware */
+	err = mlxsw_sp_mr_route_write(mr_table, mr_route, replace);
+	if (err)
+		goto err_mr_route_write;
+
 	/* Put it in the table data-structures */
 	list_add_tail(&mr_route->node, &mr_table->route_list);
 	err = rhashtable_insert_fast(&mr_table->route_ht,
@@ -423,11 +428,6 @@ int mlxsw_sp_mr_route_add(struct mlxsw_sp_mr_table *mr_table,
 	if (err)
 		goto err_rhashtable_insert;
 
-	/* Write the route to the hardware */
-	err = mlxsw_sp_mr_route_write(mr_table, mr_route, replace);
-	if (err)
-		goto err_mr_route_write;
-
 	/* Destroy the original route */
 	if (replace) {
 		rhashtable_remove_fast(&mr_table->route_ht,
@@ -440,11 +440,10 @@ int mlxsw_sp_mr_route_add(struct mlxsw_sp_mr_table *mr_table,
 	mlxsw_sp_mr_mfc_offload_update(mr_route);
 	return 0;
 
-err_mr_route_write:
-	rhashtable_remove_fast(&mr_table->route_ht, &mr_route->ht_node,
-			       mlxsw_sp_mr_route_ht_params);
 err_rhashtable_insert:
 	list_del(&mr_route->node);
+	mlxsw_sp_mr_route_erase(mr_table, mr_route);
+err_mr_route_write:
 err_no_orig_route:
 err_duplicate_route:
 	mlxsw_sp_mr_route_destroy(mr_table, mr_route);
-- 
2.24.1