Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

bridge: Fix problems around fdb entries pointing to the bridge device

Adding fdb entries pointing to the bridge device uses fdb_insert(),
which lacks various checks and does not respect added_by_user flag.

As a result, some inconsistent behavior can happen:
* Adding temporary entries succeeds but results in permanent entries.
* Same goes for "dynamic" and "use".
* Changing mac address of the bridge device causes deletion of
user-added entries.
* Replacing existing entries looks successful from userspace but actually
not, regardless of NLM_F_EXCL flag.

Use the same logic as other entries and fix them.

Fixes: 3741873b4f73 ("bridge: allow adding of fdb entries pointing to the bridge device")
Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
Acked-by: Roopa Prabhu <roopa@cumulusnetworks.com>
Signed-off-by: David S. Miller <davem@davemloft.net>

authored by

Toshiaki Makita and committed by
David S. Miller
7bb90c37 836384d2

+27 -25
+27 -25
net/bridge/br_fdb.c
··· 267 267 268 268 /* If old entry was unassociated with any port, then delete it. */ 269 269 f = __br_fdb_get(br, br->dev->dev_addr, 0); 270 - if (f && f->is_local && !f->dst) 270 + if (f && f->is_local && !f->dst && !f->added_by_user) 271 271 fdb_delete_local(br, NULL, f); 272 272 273 273 fdb_insert(br, NULL, newaddr, 0); ··· 282 282 if (!br_vlan_should_use(v)) 283 283 continue; 284 284 f = __br_fdb_get(br, br->dev->dev_addr, v->vid); 285 - if (f && f->is_local && !f->dst) 285 + if (f && f->is_local && !f->dst && !f->added_by_user) 286 286 fdb_delete_local(br, NULL, f); 287 287 fdb_insert(br, NULL, newaddr, v->vid); 288 288 } ··· 764 764 } 765 765 766 766 /* Update (create or replace) forwarding database entry */ 767 - static int fdb_add_entry(struct net_bridge_port *source, const __u8 *addr, 768 - __u16 state, __u16 flags, __u16 vid) 767 + static int fdb_add_entry(struct net_bridge *br, struct net_bridge_port *source, 768 + const __u8 *addr, __u16 state, __u16 flags, __u16 vid) 769 769 { 770 - struct net_bridge *br = source->br; 771 770 struct hlist_head *head = &br->hash[br_mac_hash(addr, vid)]; 772 771 struct net_bridge_fdb_entry *fdb; 773 772 bool modified = false; 774 773 775 774 /* If the port cannot learn allow only local and static entries */ 776 - if (!(state & NUD_PERMANENT) && !(state & NUD_NOARP) && 775 + if (source && !(state & NUD_PERMANENT) && !(state & NUD_NOARP) && 777 776 !(source->state == BR_STATE_LEARNING || 778 777 source->state == BR_STATE_FORWARDING)) 779 778 return -EPERM; 779 + 780 + if (!source && !(state & NUD_PERMANENT)) { 781 + pr_info("bridge: RTM_NEWNEIGH %s without NUD_PERMANENT\n", 782 + br->dev->name); 783 + return -EINVAL; 784 + } 780 785 781 786 fdb = fdb_find(head, addr, vid); 782 787 if (fdb == NULL) { ··· 837 832 return 0; 838 833 } 839 834 840 - static int __br_fdb_add(struct ndmsg *ndm, struct net_bridge_port *p, 841 - const unsigned char *addr, u16 nlh_flags, u16 vid) 835 + static int __br_fdb_add(struct ndmsg *ndm, struct net_bridge *br, 836 + struct net_bridge_port *p, const unsigned char *addr, 837 + u16 nlh_flags, u16 vid) 842 838 { 843 839 int err = 0; 844 840 845 841 if (ndm->ndm_flags & NTF_USE) { 842 + if (!p) { 843 + pr_info("bridge: RTM_NEWNEIGH %s with NTF_USE is not supported\n", 844 + br->dev->name); 845 + return -EINVAL; 846 + } 846 847 local_bh_disable(); 847 848 rcu_read_lock(); 848 - br_fdb_update(p->br, p, addr, vid, true); 849 + br_fdb_update(br, p, addr, vid, true); 849 850 rcu_read_unlock(); 850 851 local_bh_enable(); 851 852 } else { 852 - spin_lock_bh(&p->br->hash_lock); 853 - err = fdb_add_entry(p, addr, ndm->ndm_state, 853 + spin_lock_bh(&br->hash_lock); 854 + err = fdb_add_entry(br, p, addr, ndm->ndm_state, 854 855 nlh_flags, vid); 855 - spin_unlock_bh(&p->br->hash_lock); 856 + spin_unlock_bh(&br->hash_lock); 856 857 } 857 858 858 859 return err; ··· 895 884 dev->name); 896 885 return -EINVAL; 897 886 } 887 + br = p->br; 898 888 vg = nbp_vlan_group(p); 899 889 } 900 890 ··· 907 895 } 908 896 909 897 /* VID was specified, so use it. */ 910 - if (dev->priv_flags & IFF_EBRIDGE) 911 - err = br_fdb_insert(br, NULL, addr, vid); 912 - else 913 - err = __br_fdb_add(ndm, p, addr, nlh_flags, vid); 898 + err = __br_fdb_add(ndm, br, p, addr, nlh_flags, vid); 914 899 } else { 915 - if (dev->priv_flags & IFF_EBRIDGE) 916 - err = br_fdb_insert(br, NULL, addr, 0); 917 - else 918 - err = __br_fdb_add(ndm, p, addr, nlh_flags, 0); 900 + err = __br_fdb_add(ndm, br, p, addr, nlh_flags, 0); 919 901 if (err || !vg || !vg->num_vlans) 920 902 goto out; 921 903 ··· 920 914 list_for_each_entry(v, &vg->vlan_list, vlist) { 921 915 if (!br_vlan_should_use(v)) 922 916 continue; 923 - if (dev->priv_flags & IFF_EBRIDGE) 924 - err = br_fdb_insert(br, NULL, addr, v->vid); 925 - else 926 - err = __br_fdb_add(ndm, p, addr, nlh_flags, 927 - v->vid); 917 + err = __br_fdb_add(ndm, br, p, addr, nlh_flags, v->vid); 928 918 if (err) 929 919 goto out; 930 920 }