Ido Schimmel
2021-Aug-10 06:40 UTC
[Bridge] [PATCH net] net: bridge: validate the NUD_PERMANENT bit when adding an extern_learn FDB entry
On Mon, Aug 09, 2021 at 06:33:30PM +0300, Nikolay Aleksandrov wrote:> TBH, I want to keep that error so middle ground would be to handle NUD_PERMANENT only > when used with !p and keep it. :) WDYT ?Yes, works for me> > Solution which forces BR_FDB_LOCAL for !p calls (completely untested):Reviewed-by: Ido Schimmel <idosch at nvidia.com> Tested-by: Ido Schimmel <idosch at nvidia.com>> diff --git a/net/bridge/br.c b/net/bridge/br.c > index c8ae823aa8e7..d3a32c6813e0 100644 > --- a/net/bridge/br.c > +++ b/net/bridge/br.c > @@ -166,8 +166,7 @@ static int br_switchdev_event(struct notifier_block *unused, > case SWITCHDEV_FDB_ADD_TO_BRIDGE: > fdb_info = ptr; > err = br_fdb_external_learn_add(br, p, fdb_info->addr, > - fdb_info->vid, > - fdb_info->is_local, false); > + fdb_info->vid, false); > if (err) { > err = notifier_from_errno(err); > break; > diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c > index b8e22057f680..4e3b1b66f132 100644 > --- a/net/bridge/br_fdb.c > +++ b/net/bridge/br_fdb.c > @@ -1255,15 +1255,7 @@ static int __br_fdb_add(struct ndmsg *ndm, struct net_bridge *br, > rcu_read_unlock(); > local_bh_enable(); > } else if (ndm->ndm_flags & NTF_EXT_LEARNED) { > - if (!p && !(ndm->ndm_state & NUD_PERMANENT)) { > - NL_SET_ERR_MSG_MOD(extack, > - "FDB entry towards bridge must be permanent"); > - return -EINVAL; > - } > - > - err = br_fdb_external_learn_add(br, p, addr, vid, > - ndm->ndm_state & NUD_PERMANENT, > - true); > + err = br_fdb_external_learn_add(br, p, addr, vid, true); > } else { > spin_lock_bh(&br->hash_lock); > err = fdb_add_entry(br, p, addr, ndm, nlh_flags, vid, nfea_tb); > @@ -1491,7 +1483,7 @@ void br_fdb_unsync_static(struct net_bridge *br, struct net_bridge_port *p) > } > > int br_fdb_external_learn_add(struct net_bridge *br, struct net_bridge_port *p, > - const unsigned char *addr, u16 vid, bool is_local, > + const unsigned char *addr, u16 vid, > bool swdev_notify) > { > struct net_bridge_fdb_entry *fdb; > @@ -1509,7 +1501,7 @@ int br_fdb_external_learn_add(struct net_bridge *br, struct net_bridge_port *p, > if (swdev_notify) > flags |= BIT(BR_FDB_ADDED_BY_USER); > > - if (is_local) > + if (!p) > flags |= BIT(BR_FDB_LOCAL); > > fdb = fdb_create(br, p, addr, vid, flags); > @@ -1538,7 +1530,7 @@ int br_fdb_external_learn_add(struct net_bridge *br, struct net_bridge_port *p, > if (swdev_notify) > set_bit(BR_FDB_ADDED_BY_USER, &fdb->flags); > > - if (is_local) > + if (!p) > set_bit(BR_FDB_LOCAL, &fdb->flags); > > if (modified) > diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h > index 86969d1bd036..907e5742b392 100644 > --- a/net/bridge/br_private.h > +++ b/net/bridge/br_private.h > @@ -778,7 +778,7 @@ int br_fdb_get(struct sk_buff *skb, struct nlattr *tb[], struct net_device *dev, > int br_fdb_sync_static(struct net_bridge *br, struct net_bridge_port *p); > void br_fdb_unsync_static(struct net_bridge *br, struct net_bridge_port *p); > int br_fdb_external_learn_add(struct net_bridge *br, struct net_bridge_port *p, > - const unsigned char *addr, u16 vid, bool is_local, > + const unsigned char *addr, u16 vid, > bool swdev_notify); > int br_fdb_external_learn_del(struct net_bridge *br, struct net_bridge_port *p, > const unsigned char *addr, u16 vid, > >
Nikolay Aleksandrov
2021-Aug-10 07:21 UTC
[Bridge] [PATCH net] net: bridge: fix flags interpretation for extern learn fdb entries
From: Nikolay Aleksandrov <nikolay at nvidia.com>
Ignore fdb flags when adding port extern learn entries and always set
BR_FDB_LOCAL flag when adding bridge extern learn entries. This is
closest to the behaviour we had before and avoids breaking any use cases
which were allowed.
This patch fixes iproute2 calls which assume NUD_PERMANENT and were
allowed before, example:
$ bridge fdb add 00:11:22:33:44:55 dev swp1 extern_learn
Extern learn entries are allowed to roam, but do not expire, so static
or dynamic flags make no sense for them.
Fixes: eb100e0e24a2 ("net: bridge: allow to add externally learned entries
from user-space")
Fixes: 0541a6293298 ("net: bridge: validate the NUD_PERMANENT bit when
adding an extern_learn FDB entry")
Reviewed-by: Ido Schimmel <idosch at nvidia.com>
Tested-by: Ido Schimmel <idosch at nvidia.com>
Signed-off-by: Nikolay Aleksandrov <nikolay at nvidia.com>
---
As discussed I decided to keep the error for !p and !NUD_PERMANENT case.
net/bridge/br.c | 3 +--
net/bridge/br_fdb.c | 11 ++++-------
net/bridge/br_private.h | 2 +-
3 files changed, 6 insertions(+), 10 deletions(-)
diff --git a/net/bridge/br.c b/net/bridge/br.c
index bbab9984f24e..ef743f94254d 100644
--- a/net/bridge/br.c
+++ b/net/bridge/br.c
@@ -166,8 +166,7 @@ static int br_switchdev_event(struct notifier_block *unused,
case SWITCHDEV_FDB_ADD_TO_BRIDGE:
fdb_info = ptr;
err = br_fdb_external_learn_add(br, p, fdb_info->addr,
- fdb_info->vid,
- fdb_info->is_local, false);
+ fdb_info->vid, false);
if (err) {
err = notifier_from_errno(err);
break;
diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
index 835cec1e5a03..5dee30966ed3 100644
--- a/net/bridge/br_fdb.c
+++ b/net/bridge/br_fdb.c
@@ -1044,10 +1044,7 @@ static int __br_fdb_add(struct ndmsg *ndm, struct
net_bridge *br,
"FDB entry towards bridge must be permanent");
return -EINVAL;
}
-
- err = br_fdb_external_learn_add(br, p, addr, vid,
- ndm->ndm_state & NUD_PERMANENT,
- true);
+ err = br_fdb_external_learn_add(br, p, addr, vid, true);
} else {
spin_lock_bh(&br->hash_lock);
err = fdb_add_entry(br, p, addr, ndm, nlh_flags, vid, nfea_tb);
@@ -1275,7 +1272,7 @@ void br_fdb_unsync_static(struct net_bridge *br, struct
net_bridge_port *p)
}
int br_fdb_external_learn_add(struct net_bridge *br, struct net_bridge_port *p,
- const unsigned char *addr, u16 vid, bool is_local,
+ const unsigned char *addr, u16 vid,
bool swdev_notify)
{
struct net_bridge_fdb_entry *fdb;
@@ -1293,7 +1290,7 @@ int br_fdb_external_learn_add(struct net_bridge *br,
struct net_bridge_port *p,
if (swdev_notify)
flags |= BIT(BR_FDB_ADDED_BY_USER);
- if (is_local)
+ if (!p)
flags |= BIT(BR_FDB_LOCAL);
fdb = fdb_create(br, p, addr, vid, flags);
@@ -1322,7 +1319,7 @@ int br_fdb_external_learn_add(struct net_bridge *br,
struct net_bridge_port *p,
if (swdev_notify)
set_bit(BR_FDB_ADDED_BY_USER, &fdb->flags);
- if (is_local)
+ if (!p)
set_bit(BR_FDB_LOCAL, &fdb->flags);
if (modified)
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index aa64d8d63ca3..2b48b204205e 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -711,7 +711,7 @@ int br_fdb_get(struct sk_buff *skb, struct nlattr *tb[],
struct net_device *dev,
int br_fdb_sync_static(struct net_bridge *br, struct net_bridge_port *p);
void br_fdb_unsync_static(struct net_bridge *br, struct net_bridge_port *p);
int br_fdb_external_learn_add(struct net_bridge *br, struct net_bridge_port *p,
- const unsigned char *addr, u16 vid, bool is_local,
+ const unsigned char *addr, u16 vid,
bool swdev_notify);
int br_fdb_external_learn_del(struct net_bridge *br, struct net_bridge_port *p,
const unsigned char *addr, u16 vid,
--
2.31.1