On 4/8/25 6:30 PM, Matthias Schiffer wrote:
batadv_check_known_mac_addr() is both too lenient and too strict:
- It is called from batadv_hardif_add_interface(), which means that it
checked interfaces that are not used for batman-adv at all. Move it
to batadv_hardif_enable_interface(). Also, restrict it to hardifs of
the same mesh interface; different mesh interfaces should not interact
at all. The batadv_check_known_mac_addr() argument is changed from
`struct net_device` to `struct batadv_hard_iface` to achieve this.
- The check only cares about hardifs in BATADV_IF_ACTIVE and
BATADV_IF_TO_BE_ACTIVATED states, but interfaces in BATADV_IF_INACTIVE
state should be checked as well, or the following steps will not
result in a warning then they should:
- Add two interfaces on down state with different MAC addresses to
a mesh as hardifs
- Change the MAC addresses so they confliect
- Set interfaces to up state
Now there will be two active hardifs with the same MAC address, but no
warning. Fix by only ignoring hardifs in BATADV_IF_NOT_IN_USE state.
The RCU lock can be dropped, as we're holding RTNL anyways when the
function is called.
While we're at it, also switch from pr_warn() to netdev_warn().
Fixes: c6c8fea29769 ("net: Add batman-adv meshing protocol")
Signed-off-by: Matthias Schiffer <mschiffer@xxxxxxxxxxxxxxxxxxxx>
Even if marked for net I assume this will eventually go first via the
batman tree.
---
Aside: batadv_hardif_add_interface() being called for all existing
interfaces and having a global batadv_hardif_list at all is also not
very nice, but this will be addressed separately, as changing it will
require more refactoring.
net/batman-adv/hard-interface.c | 37 ++++++++++++++++++++-------------
1 file changed, 22 insertions(+), 15 deletions(-)
diff --git a/net/batman-adv/hard-interface.c b/net/batman-adv/hard-interface.c
index f145f9662653..07b436626afb 100644
--- a/net/batman-adv/hard-interface.c
+++ b/net/batman-adv/hard-interface.c
@@ -506,28 +506,34 @@ batadv_hardif_is_iface_up(const struct batadv_hard_iface *hard_iface)
return false;
}
-static void batadv_check_known_mac_addr(const struct net_device *net_dev)
+static void batadv_check_known_mac_addr(const struct batadv_hard_iface *hard_iface)
{
- const struct batadv_hard_iface *hard_iface;
+ const struct net_device *mesh_iface = hard_iface->mesh_iface;
+ const struct batadv_hard_iface *tmp_hard_iface;
- rcu_read_lock();
- list_for_each_entry_rcu(hard_iface, &batadv_hardif_list, list) {
- if (hard_iface->if_status != BATADV_IF_ACTIVE &&
- hard_iface->if_status != BATADV_IF_TO_BE_ACTIVATED)
+ if (!mesh_iface)
+ return;
+
+ list_for_each_entry(tmp_hard_iface, &batadv_hardif_list, list) {
+ if (tmp_hard_iface == hard_iface)
+ continue;
+
+ if (tmp_hard_iface->mesh_iface != mesh_iface)
continue;
- if (hard_iface->net_dev == net_dev)
+ if (tmp_hard_iface->if_status == BATADV_IF_NOT_IN_USE)
continue;
- if (!batadv_compare_eth(hard_iface->net_dev->dev_addr,
- net_dev->dev_addr))
+ if (!batadv_compare_eth(tmp_hard_iface->net_dev->dev_addr,
+ hard_iface->net_dev->dev_addr))
continue;
- pr_warn("The newly added mac address (%pM) already exists on: %s\n",
- net_dev->dev_addr, hard_iface->net_dev->name);
- pr_warn("It is strongly recommended to keep mac addresses unique to avoid problems!\n");
+ netdev_warn(hard_iface->net_dev,
+ "The newly added mac address (%pM) already exists on: %s\n",
+ hard_iface->net_dev->dev_addr, tmp_hard_iface->net_dev->name);
+ netdev_warn(hard_iface->net_dev,
+ "It is strongly recommended to keep mac addresses unique to avoid problems!\n");
}
- rcu_read_unlock();
}
I feel like the above code mixes unnecessarily fix and refactor
(variable rename, different print helper usage).
I think the fix should be minimal, the refactor should land in a
different patch for next.
Attachment:
OpenPGP_signature.asc
Description: OpenPGP digital signature