lkml.org 
[lkml]   [2020]   [May]   [8]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 4.4 305/312] bridge: Fix problems around fdb entries pointing to the bridge device
    Date
    From: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>

    commit 7bb90c3715a496c650b2e879225030f9dd9cfafb upstream.

    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>
    Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

    ---
    net/bridge/br_fdb.c | 52 +++++++++++++++++++++++++++-------------------------
    1 file changed, 27 insertions(+), 25 deletions(-)

    --- a/net/bridge/br_fdb.c
    +++ b/net/bridge/br_fdb.c
    @@ -266,7 +266,7 @@ void br_fdb_change_mac_address(struct ne

    /* If old entry was unassociated with any port, then delete it. */
    f = __br_fdb_get(br, br->dev->dev_addr, 0);
    - if (f && f->is_local && !f->dst)
    + if (f && f->is_local && !f->dst && !f->added_by_user)
    fdb_delete_local(br, NULL, f);

    fdb_insert(br, NULL, newaddr, 0);
    @@ -281,7 +281,7 @@ void br_fdb_change_mac_address(struct ne
    if (!br_vlan_should_use(v))
    continue;
    f = __br_fdb_get(br, br->dev->dev_addr, v->vid);
    - if (f && f->is_local && !f->dst)
    + if (f && f->is_local && !f->dst && !f->added_by_user)
    fdb_delete_local(br, NULL, f);
    fdb_insert(br, NULL, newaddr, v->vid);
    }
    @@ -758,20 +758,25 @@ out:
    }

    /* Update (create or replace) forwarding database entry */
    -static int fdb_add_entry(struct net_bridge_port *source, const __u8 *addr,
    - __u16 state, __u16 flags, __u16 vid)
    +static int fdb_add_entry(struct net_bridge *br, struct net_bridge_port *source,
    + const __u8 *addr, __u16 state, __u16 flags, __u16 vid)
    {
    - struct net_bridge *br = source->br;
    struct hlist_head *head = &br->hash[br_mac_hash(addr, vid)];
    struct net_bridge_fdb_entry *fdb;
    bool modified = false;

    /* If the port cannot learn allow only local and static entries */
    - if (!(state & NUD_PERMANENT) && !(state & NUD_NOARP) &&
    + if (source && !(state & NUD_PERMANENT) && !(state & NUD_NOARP) &&
    !(source->state == BR_STATE_LEARNING ||
    source->state == BR_STATE_FORWARDING))
    return -EPERM;

    + if (!source && !(state & NUD_PERMANENT)) {
    + pr_info("bridge: RTM_NEWNEIGH %s without NUD_PERMANENT\n",
    + br->dev->name);
    + return -EINVAL;
    + }
    +
    fdb = fdb_find(head, addr, vid);
    if (fdb == NULL) {
    if (!(flags & NLM_F_CREATE))
    @@ -826,22 +831,28 @@ static int fdb_add_entry(struct net_brid
    return 0;
    }

    -static int __br_fdb_add(struct ndmsg *ndm, struct net_bridge_port *p,
    - const unsigned char *addr, u16 nlh_flags, u16 vid)
    +static int __br_fdb_add(struct ndmsg *ndm, struct net_bridge *br,
    + struct net_bridge_port *p, const unsigned char *addr,
    + u16 nlh_flags, u16 vid)
    {
    int err = 0;

    if (ndm->ndm_flags & NTF_USE) {
    + if (!p) {
    + pr_info("bridge: RTM_NEWNEIGH %s with NTF_USE is not supported\n",
    + br->dev->name);
    + return -EINVAL;
    + }
    local_bh_disable();
    rcu_read_lock();
    - br_fdb_update(p->br, p, addr, vid, true);
    + br_fdb_update(br, p, addr, vid, true);
    rcu_read_unlock();
    local_bh_enable();
    } else {
    - spin_lock_bh(&p->br->hash_lock);
    - err = fdb_add_entry(p, addr, ndm->ndm_state,
    + spin_lock_bh(&br->hash_lock);
    + err = fdb_add_entry(br, p, addr, ndm->ndm_state,
    nlh_flags, vid);
    - spin_unlock_bh(&p->br->hash_lock);
    + spin_unlock_bh(&br->hash_lock);
    }

    return err;
    @@ -878,6 +889,7 @@ int br_fdb_add(struct ndmsg *ndm, struct
    dev->name);
    return -EINVAL;
    }
    + br = p->br;
    vg = nbp_vlan_group(p);
    }

    @@ -889,15 +901,9 @@ int br_fdb_add(struct ndmsg *ndm, struct
    }

    /* VID was specified, so use it. */
    - if (dev->priv_flags & IFF_EBRIDGE)
    - err = br_fdb_insert(br, NULL, addr, vid);
    - else
    - err = __br_fdb_add(ndm, p, addr, nlh_flags, vid);
    + err = __br_fdb_add(ndm, br, p, addr, nlh_flags, vid);
    } else {
    - if (dev->priv_flags & IFF_EBRIDGE)
    - err = br_fdb_insert(br, NULL, addr, 0);
    - else
    - err = __br_fdb_add(ndm, p, addr, nlh_flags, 0);
    + err = __br_fdb_add(ndm, br, p, addr, nlh_flags, 0);
    if (err || !vg || !vg->num_vlans)
    goto out;

    @@ -908,11 +914,7 @@ int br_fdb_add(struct ndmsg *ndm, struct
    list_for_each_entry(v, &vg->vlan_list, vlist) {
    if (!br_vlan_should_use(v))
    continue;
    - if (dev->priv_flags & IFF_EBRIDGE)
    - err = br_fdb_insert(br, NULL, addr, v->vid);
    - else
    - err = __br_fdb_add(ndm, p, addr, nlh_flags,
    - v->vid);
    + err = __br_fdb_add(ndm, br, p, addr, nlh_flags, v->vid);
    if (err)
    goto out;
    }

    \
     
     \ /
      Last update: 2020-05-08 14:49    [W:4.270 / U:0.416 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site