| From foo@baz Tue 17 Mar 2020 11:15:44 AM CET |
| From: Sven Eckelmann <sven@narfation.org> |
| Date: Mon, 16 Mar 2020 23:31:01 +0100 |
| Subject: batman-adv: Prevent duplicated gateway_node entry |
| To: stable@vger.kernel.org |
| Cc: Sven Eckelmann <sven@narfation.org>, Marek Lindner <mareklindner@neomailbox.ch>, Simon Wunderlich <sw@simonwunderlich.de> |
| Message-ID: <20200316223105.6333-21-sven@narfation.org> |
| |
| From: Sven Eckelmann <sven@narfation.org> |
| |
| commit dff9bc42ab0b2d38c5e90ddd79b238fed5b4c7ad upstream. |
| |
| The function batadv_gw_node_add is responsible for adding new gw_node to |
| the gateway_list. It is expecting that the caller already checked that |
| there is not already an entry with the same key or not. |
| |
| But the lock for the list is only held when the list is really modified. |
| This could lead to duplicated entries because another context could create |
| an entry with the same key between the check and the list manipulation. |
| |
| The check and the manipulation of the list must therefore be in the same |
| locked code section. |
| |
| Fixes: c6c8fea29769 ("net: Add batman-adv meshing protocol") |
| Signed-off-by: Sven Eckelmann <sven@narfation.org> |
| Acked-by: Marek Lindner <mareklindner@neomailbox.ch> |
| Signed-off-by: Simon Wunderlich <sw@simonwunderlich.de> |
| Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> |
| --- |
| net/batman-adv/gateway_client.c | 11 +++++++++-- |
| 1 file changed, 9 insertions(+), 2 deletions(-) |
| |
| --- a/net/batman-adv/gateway_client.c |
| +++ b/net/batman-adv/gateway_client.c |
| @@ -31,6 +31,7 @@ |
| #include <linux/kernel.h> |
| #include <linux/kref.h> |
| #include <linux/list.h> |
| +#include <linux/lockdep.h> |
| #include <linux/netdevice.h> |
| #include <linux/netlink.h> |
| #include <linux/rculist.h> |
| @@ -325,6 +326,9 @@ out: |
| * @bat_priv: the bat priv with all the soft interface information |
| * @orig_node: originator announcing gateway capabilities |
| * @gateway: announced bandwidth information |
| + * |
| + * Has to be called with the appropriate locks being acquired |
| + * (gw.list_lock). |
| */ |
| static void batadv_gw_node_add(struct batadv_priv *bat_priv, |
| struct batadv_orig_node *orig_node, |
| @@ -332,6 +336,8 @@ static void batadv_gw_node_add(struct ba |
| { |
| struct batadv_gw_node *gw_node; |
| |
| + lockdep_assert_held(&bat_priv->gw.list_lock); |
| + |
| if (gateway->bandwidth_down == 0) |
| return; |
| |
| @@ -346,10 +352,8 @@ static void batadv_gw_node_add(struct ba |
| gw_node->bandwidth_down = ntohl(gateway->bandwidth_down); |
| gw_node->bandwidth_up = ntohl(gateway->bandwidth_up); |
| |
| - spin_lock_bh(&bat_priv->gw.list_lock); |
| kref_get(&gw_node->refcount); |
| hlist_add_head_rcu(&gw_node->list, &bat_priv->gw.list); |
| - spin_unlock_bh(&bat_priv->gw.list_lock); |
| |
| batadv_dbg(BATADV_DBG_BATMAN, bat_priv, |
| "Found new gateway %pM -> gw bandwidth: %u.%u/%u.%u MBit\n", |
| @@ -404,11 +408,14 @@ void batadv_gw_node_update(struct batadv |
| { |
| struct batadv_gw_node *gw_node, *curr_gw = NULL; |
| |
| + spin_lock_bh(&bat_priv->gw.list_lock); |
| gw_node = batadv_gw_node_get(bat_priv, orig_node); |
| if (!gw_node) { |
| batadv_gw_node_add(bat_priv, orig_node, gateway); |
| + spin_unlock_bh(&bat_priv->gw.list_lock); |
| goto out; |
| } |
| + spin_unlock_bh(&bat_priv->gw.list_lock); |
| |
| if ((gw_node->bandwidth_down == ntohl(gateway->bandwidth_down)) && |
| (gw_node->bandwidth_up == ntohl(gateway->bandwidth_up))) |