netdev - [PATCH net 4/7] bridge: Fix the way checking if a local fdb entry can be deleted

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: <1385966439-26526-5-git-send-email-makita.toshiaki@lab.ntt.co.jp>
Date:	Mon,  2 Dec 2013 15:40:35 +0900
From:	Toshiaki Makita <makita.toshiaki@....ntt.co.jp>
To:	"David S . Miller" <davem@...emloft.net>,
	Stephen Hemminger <stephen@...workplumber.org>,
	Vlad Yasevich <vyasevic@...hat.com>, netdev@...r.kernel.org
Cc:	Toshiaki Makita <makita.toshiaki@....ntt.co.jp>
Subject: [PATCH net 4/7] bridge: Fix the way checking if a local fdb entry can be deleted

We should take into account the followings when deleting a local fdb entry.

- nbp_vlan_find() can be used only when vid != 0 to check if an entry is
  deletable, because a fdb entry with vid 0 can exist at any time but
  nbp_vlan_find() always return false with vid 0.

  Example of problematic case:
    ip link set eth0 address 12:34:56:78:90:ab
    ip link set eth1 address 12:34:56:78:90:ab
    brctl addif br0 eth0
    brctl addif br0 eth1
    ip link set eth0 address aa:bb:cc:dd:ee:ff
  Then, the fdb entry 12:34:56:78:90:ab will be deleted even though the
  bridge port eth1 still has that address.

- The port to which the bridge device is attached might needs a local entry
  if its mac address is set manually.

  Example of problematic case:
    ip link set eth0 address 12:34:56:78:90:ab
    brctl addif br0 eth0
    ip link set br0 address 12:34:56:78:90:ab
    ip link set eth0 address aa:bb:cc:dd:ee:ff
  Then, the fdb still must have the entry 12:34:56:78:90:ab, but it will be
  deleted.

We can use br->dev->addr_assign_type to check if the address is manually
set or not, but I propose another approach.

Since we delete and insert local entries whenever changing mac address
of the bridge device, we can change dst of the entry to NULL regardless of
addr_assign_type when deleting an entry associated with a certain port,
and if it is found to be unnecessary later, then delete it.
That is, if changing mac address of a port, the entry might be changed
to its dst being NULL first, but is eventually deleted when recalculating
and changing bridge id.

This approach is useful when we want to share the code with deleting
vlan in which the bridge device might want such an entry regardless of
addr_assign_type, and makes things easy because we don't have to consider
if mac address of the bridge device will be changed or not at
fdb_delete_local().

Signed-off-by: Toshiaki Makita <makita.toshiaki@....ntt.co.jp>
---
 net/bridge/br_fdb.c     |  9 ++++++++-
 net/bridge/br_private.h |  6 ++++++
 net/bridge/br_vlan.c    | 19 +++++++++++++++++++
 3 files changed, 33 insertions(+), 1 deletion(-)

diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
index d29e184..2c79b1b 100644
--- a/net/bridge/br_fdb.c
+++ b/net/bridge/br_fdb.c
@@ -108,12 +108,19 @@ static void fdb_delete_local(struct net_bridge *br,
 	/* Maybe another port has same hw addr? */
 	list_for_each_entry(op, &br->port_list, list) {
 		if (op != p && ether_addr_equal(op->dev->dev_addr, addr) &&
-		    nbp_vlan_find(op, vid)) {
+		    (!vid || nbp_vlan_find(op, vid))) {
 			f->dst = op;
 			return;
 		}
 	}
 
+	/* Maybe bridge device has same hw addr? */
+	if (p && ether_addr_equal(br->dev->dev_addr, addr) &&
+	    (!vid || br_vlan_find(br, vid))) {
+		f->dst = NULL;
+		return;
+	}
+
 	fdb_delete(br, f);
 }
 
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index 0902658..673ef9d 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -584,6 +584,7 @@ struct sk_buff *br_handle_vlan(struct net_bridge *br,
 int br_vlan_add(struct net_bridge *br, u16 vid, u16 flags);
 int br_vlan_delete(struct net_bridge *br, u16 vid);
 void br_vlan_flush(struct net_bridge *br);
+bool br_vlan_find(struct net_bridge *br, u16 vid);
 int br_vlan_filter_toggle(struct net_bridge *br, unsigned long val);
 int nbp_vlan_add(struct net_bridge_port *port, u16 vid, u16 flags);
 int nbp_vlan_delete(struct net_bridge_port *port, u16 vid);
@@ -665,6 +666,11 @@ static inline void br_vlan_flush(struct net_bridge *br)
 {
 }
 
+static inline bool br_vlan_find(struct net_bridge *br, u16 vid)
+{
+	return false;
+}
+
 static inline int nbp_vlan_add(struct net_bridge_port *port, u16 vid, u16 flags)
 {
 	return -EOPNOTSUPP;
diff --git a/net/bridge/br_vlan.c b/net/bridge/br_vlan.c
index af5ebd1..f87ab88f 100644
--- a/net/bridge/br_vlan.c
+++ b/net/bridge/br_vlan.c
@@ -316,6 +316,25 @@ void br_vlan_flush(struct net_bridge *br)
 	__vlan_flush(pv);
 }
 
+bool br_vlan_find(struct net_bridge *br, u16 vid)
+{
+	struct net_port_vlans *pv;
+	bool found = false;
+
+	rcu_read_lock();
+	pv = rcu_dereference(br->vlan_info);
+
+	if (!pv)
+		goto out;
+
+	if (test_bit(vid, pv->vlan_bitmap))
+		found = true;
+
+out:
+	rcu_read_unlock();
+	return found;
+}
+
 int br_vlan_filter_toggle(struct net_bridge *br, unsigned long val)
 {
 	if (!rtnl_trylock())
-- 
1.8.1.2

--
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