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
| ||
|
Message-ID: <e0b3861a-18eb-80ad-1cf8-c563f97e2157@blackwall.org> Date: Thu, 14 Sep 2023 18:15:23 +0300 From: Nikolay Aleksandrov <razor@...ckwall.org> To: Johannes Nixdorf <jnixdorf-oss@....de>, "David S. Miller" <davem@...emloft.net>, Andrew Lunn <andrew@...n.ch>, David Ahern <dsahern@...il.com>, Eric Dumazet <edumazet@...gle.com>, Florian Fainelli <f.fainelli@...il.com>, Ido Schimmel <idosch@...dia.com>, Jakub Kicinski <kuba@...nel.org>, Oleksij Rempel <linux@...pel-privat.de>, Paolo Abeni <pabeni@...hat.com>, Roopa Prabhu <roopa@...dia.com>, Shuah Khan <shuah@...nel.org>, Vladimir Oltean <vladimir.oltean@....com> Cc: bridge@...ts.linux-foundation.org, netdev@...r.kernel.org, linux-kernel@...r.kernel.org, linux-kselftest@...r.kernel.org Subject: Re: [PATCH net-next v3 3/6] net: bridge: Track and limit dynamically learned FDB entries On 9/5/23 14:47, Johannes Nixdorf wrote: > A malicious actor behind one bridge port may spam the kernel with packets > with a random source MAC address, each of which will create an FDB entry, > each of which is a dynamic allocation in the kernel. > > There are roughly 2^48 different MAC addresses, further limited by the > rhashtable they are stored in to 2^31. Each entry is of the type struct > net_bridge_fdb_entry, which is currently 128 bytes big. This means the > maximum amount of memory allocated for FDB entries is 2^31 * 128B = > 256GiB, which is too much for most computers. > > Mitigate this by maintaining a per bridge count of those automatically > generated entries in fdb_n_learned_entries, and a limit in > fdb_max_learned_entries. If the limit is hit new entries are not learned > anymore. > > For backwards compatibility the default setting of 0 disables the limit. > > User-added entries by netlink or from bridge or bridge port addresses > are never blocked and do not count towards that limit. > > Introduce a new fdb entry flag BR_FDB_DYNAMIC_LEARNED to keep track of > whether an FDB entry is included in the count. The flag is enabled for > dynamically learned entries, and disabled for all other entries. This > should be equivalent to BR_FDB_ADDED_BY_USER and BR_FDB_LOCAL being unset, > but contrary to the two flags it can be toggled atomically. > > Atomicity is required here, as there are multiple callers that modify the > flags, but are not under a common lock (br_fdb_update is the exception > for br->hash_lock, br_fdb_external_learn_add for RTNL). > > Signed-off-by: Johannes Nixdorf <jnixdorf-oss@....de> > --- > net/bridge/br_fdb.c | 34 ++++++++++++++++++++++++++++++++-- > net/bridge/br_private.h | 4 ++++ > 2 files changed, 36 insertions(+), 2 deletions(-) > > diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c > index 06e28ef8d9ff..f8a96ed9a338 100644 > --- a/net/bridge/br_fdb.c > +++ b/net/bridge/br_fdb.c > @@ -329,11 +329,18 @@ static void fdb_delete(struct net_bridge *br, struct net_bridge_fdb_entry *f, > hlist_del_init_rcu(&f->fdb_node); > rhashtable_remove_fast(&br->fdb_hash_tbl, &f->rhnode, > br_fdb_rht_params); > + if (test_bit(BR_FDB_DYNAMIC_LEARNED, &f->flags)) this is racy with br_fdb_update(), the fdb entry is still visible because a grace period hasn't passed, so in theory br_fdb_update() can unset the bit after this test and... > + atomic_dec(&br->fdb_n_learned_entries); ... this will decrease once more wrongly > fdb_notify(br, f, RTM_DELNEIGH, swdev_notify); > call_rcu(&f->rcu, fdb_rcu_free); > } > > -/* Delete a local entry if no other port had the same address. */ > +/* Delete a local entry if no other port had the same address. > + * > + * This function should only be called on entries with BR_FDB_LOCAL set, > + * so even with BR_FDB_ADDED_BY_USER cleared we never need to increase > + * the accounting for dynamically learned entries again. > + */ > static void fdb_delete_local(struct net_bridge *br, > const struct net_bridge_port *p, > struct net_bridge_fdb_entry *f) > @@ -388,9 +395,20 @@ static struct net_bridge_fdb_entry *fdb_create(struct net_bridge *br, > __u16 vid, > unsigned long flags) > { > + bool learned = !test_bit(BR_FDB_ADDED_BY_USER, &flags) && > + !test_bit(BR_FDB_LOCAL, &flags); > + u32 max_learned = READ_ONCE(br->fdb_max_learned_entries); > struct net_bridge_fdb_entry *fdb; > int err; > > + if (likely(learned)) { > + int n_learned = atomic_read(&br->fdb_n_learned_entries); > + > + if (unlikely(max_learned && n_learned >= max_learned)) > + return NULL; > + __set_bit(BR_FDB_DYNAMIC_LEARNED, &flags); > + } > + > fdb = kmem_cache_alloc(br_fdb_cache, GFP_ATOMIC); > if (!fdb) > return NULL; > @@ -407,6 +425,9 @@ static struct net_bridge_fdb_entry *fdb_create(struct net_bridge *br, > return NULL; > } > > + if (likely(learned)) > + atomic_inc(&br->fdb_n_learned_entries); > + > hlist_add_head_rcu(&fdb->fdb_node, &br->fdb_list); > > return fdb; > @@ -893,8 +914,11 @@ void br_fdb_update(struct net_bridge *br, struct net_bridge_port *source, > clear_bit(BR_FDB_LOCKED, &fdb->flags); > } > > - if (unlikely(test_bit(BR_FDB_ADDED_BY_USER, &flags))) > + if (unlikely(test_bit(BR_FDB_ADDED_BY_USER, &flags))) { > set_bit(BR_FDB_ADDED_BY_USER, &fdb->flags); > + if (test_and_clear_bit(BR_FDB_DYNAMIC_LEARNED, &fdb->flags)) > + atomic_dec(&br->fdb_n_learned_entries); > + } > if (unlikely(fdb_modified)) { > trace_br_fdb_update(br, source, addr, vid, flags); > fdb_notify(br, fdb, RTM_NEWNEIGH, true); > @@ -1071,6 +1095,8 @@ static int fdb_add_entry(struct net_bridge *br, struct net_bridge_port *source, > } > > set_bit(BR_FDB_ADDED_BY_USER, &fdb->flags); > + if (test_and_clear_bit(BR_FDB_DYNAMIC_LEARNED, &fdb->flags)) > + atomic_dec(&br->fdb_n_learned_entries); > } > > if (fdb_to_nud(br, fdb) != state) { > @@ -1445,6 +1471,10 @@ int br_fdb_external_learn_add(struct net_bridge *br, struct net_bridge_port *p, > if (!p) > set_bit(BR_FDB_LOCAL, &fdb->flags); > > + if ((swdev_notify || !p) && > + test_and_clear_bit(BR_FDB_DYNAMIC_LEARNED, &fdb->flags)) > + atomic_dec(&br->fdb_n_learned_entries); > + > if (modified) > fdb_notify(br, fdb, RTM_NEWNEIGH, swdev_notify); > } > diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h > index a63b32c1638e..675cc40ae1dc 100644 > --- a/net/bridge/br_private.h > +++ b/net/bridge/br_private.h > @@ -274,6 +274,7 @@ enum { > BR_FDB_NOTIFY, > BR_FDB_NOTIFY_INACTIVE, > BR_FDB_LOCKED, > + BR_FDB_DYNAMIC_LEARNED, > }; > > struct net_bridge_fdb_key { > @@ -554,6 +555,9 @@ struct net_bridge { > struct kobject *ifobj; > u32 auto_cnt; > > + atomic_t fdb_n_learned_entries; > + u32 fdb_max_learned_entries; > + > #ifdef CONFIG_NET_SWITCHDEV > /* Counter used to make sure that hardware domains get unique > * identifiers in case a bridge spans multiple switchdev instances. >
Powered by blists - more mailing lists