| 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: <530E23A1.8030601@redhat.com>
Date: Wed, 26 Feb 2014 12:25:53 -0500
From: Vlad Yasevich <vyasevic@...hat.com>
To: "Michael S. Tsirkin" <mst@...hat.com>
CC: netdev@...r.kernel.org, bridge@...ts.linux-foundation.org,
shemminger@...tta.com, jhs@...atatu.com, john.r.fastabend@...el.com
Subject: Re: [PATCH 3/7] bridge: Add addresses from static fdbs to bridge
address list
On 02/26/2014 11:23 AM, Michael S. Tsirkin wrote:
> On Wed, Feb 26, 2014 at 10:18:21AM -0500, Vlad Yasevich wrote:
>> When a static fdb entry is created, add the mac address to the bridge
>> address list. This list is used to program the proper port's
>> address list.
>>
>> Signed-off-by: Vlad Yasevich <vyasevic@...hat.com>
>
> Hmm won't this mean learned addresses are missing there?
Learning should be disabled on interfaces that disable flood.
The combination of learning without flood is problematic in
cases where fdbs timeout before arp entries do.
In such cases, arp will first try unicasts to validate the neighbor
and these would be dropped.
I've been thinking about automatically turning off learning
when flood is turned off. What I don't like about it is
that it needs extra state to reverse the process.
We could log a warning, but that requires people to read logs...
> And if so isn't this a problem when we use this list
> in the next patch?
>
> I guess we could limit this to configurations where
> learning is also disabled (not just flood) -
> seems a bit arbitrary but will likely cover
> most use-cases.
>
Right. From the start we wanted manual configuration here.
Doing this with learning will open us to remote mac filter
exhaustion attacks.
-vlad
>
>> ---
>> include/linux/netdevice.h | 6 +++
>> net/bridge/br_device.c | 2 +
>> net/bridge/br_fdb.c | 110 +++++++++++++++++++++++++++++++++++++++++++---
>> net/bridge/br_if.c | 14 ++++--
>> net/bridge/br_private.h | 3 ++
>> net/core/dev.c | 1 +
>> net/core/dev_addr_lists.c | 14 +++---
>> 7 files changed, 134 insertions(+), 16 deletions(-)
>>
>> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
>> index 440a02e..e29cce1 100644
>> --- a/include/linux/netdevice.h
>> +++ b/include/linux/netdevice.h
>> @@ -2881,6 +2881,12 @@ int register_netdev(struct net_device *dev);
>> void unregister_netdev(struct net_device *dev);
>>
>> /* General hardware address lists handling functions */
>> +int __hw_addr_add(struct netdev_hw_addr_list *list,
>> + const unsigned char *addr, int addr_len,
>> + unsigned char addr_type);
>> +int __hw_addr_del(struct netdev_hw_addr_list *list,
>> + const unsigned char *addr, int addr_len,
>> + unsigned char addr_type);
>> int __hw_addr_sync(struct netdev_hw_addr_list *to_list,
>> struct netdev_hw_addr_list *from_list, int addr_len);
>> void __hw_addr_unsync(struct netdev_hw_addr_list *to_list,
>> diff --git a/net/bridge/br_device.c b/net/bridge/br_device.c
>> index 63f0455..1521db6 100644
>> --- a/net/bridge/br_device.c
>> +++ b/net/bridge/br_device.c
>> @@ -100,6 +100,8 @@ static int br_dev_init(struct net_device *dev)
>> u64_stats_init(&br_dev_stats->syncp);
>> }
>>
>> + __hw_addr_init(&br->conf_addrs);
>> +
>> return 0;
>> }
>>
>> diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
>> index 9203d5a..ef95e81 100644
>> --- a/net/bridge/br_fdb.c
>> +++ b/net/bridge/br_fdb.c
>> @@ -35,6 +35,9 @@ static int fdb_insert(struct net_bridge *br, struct net_bridge_port *source,
>> static void fdb_notify(struct net_bridge *br,
>> const struct net_bridge_fdb_entry *, int);
>>
>> +static int br_addr_add(struct net_bridge *br, const unsigned char *addr);
>> +static int br_addr_del(struct net_bridge *br, const unsigned char *addr);
>> +
>> static u32 fdb_salt __read_mostly;
>>
>> int __init br_fdb_init(void)
>> @@ -87,6 +90,9 @@ static void fdb_rcu_free(struct rcu_head *head)
>>
>> static void fdb_delete(struct net_bridge *br, struct net_bridge_fdb_entry *f)
>> {
>> + if (f->is_static)
>> + br_addr_del(br, f->addr.addr);
>> +
>> hlist_del_rcu(&f->hlist);
>> fdb_notify(br, f, RTM_DELNEIGH);
>> call_rcu(&f->rcu, fdb_rcu_free);
>> @@ -466,6 +472,7 @@ static int fdb_insert(struct net_bridge *br, struct net_bridge_port *source,
>> return -ENOMEM;
>>
>> fdb->is_local = fdb->is_static = 1;
>> + br_addr_add(br, addr);
>> fdb_notify(br, fdb, RTM_NEWNEIGH);
>> return 0;
>> }
>> @@ -678,16 +685,29 @@ static int fdb_add_entry(struct net_bridge_port *source, const __u8 *addr,
>> }
>>
>> if (fdb_to_nud(fdb) != state) {
>> - if (state & NUD_PERMANENT)
>> - fdb->is_local = fdb->is_static = 1;
>> - else if (state & NUD_NOARP) {
>> + if (state & NUD_PERMANENT) {
>> + fdb->is_local = 1;
>> + if (!fdb->is_static) {
>> + fdb->is_static = 1;
>> + br_addr_add(br, addr);
>> + }
>> + } else if (state & NUD_NOARP) {
>> + fdb->is_local = 0;
>> + if (!fdb->is_static) {
>> + fdb->is_static = 1;
>> + br_addr_add(br, addr);
>> + }
>> + } else {
>> fdb->is_local = 0;
>> - fdb->is_static = 1;
>> - } else
>> - fdb->is_local = fdb->is_static = 0;
>> + if (fdb->is_static) {
>> + fdb->is_static = 0;
>> + br_addr_del(br, addr);
>> + }
>> + }
>>
>> modified = true;
>> }
>> +
>> fdb->added_by_user = 1;
>>
>> fdb->used = jiffies;
>> @@ -874,3 +894,81 @@ int br_fdb_delete(struct ndmsg *ndm, struct nlattr *tb[],
>> out:
>> return err;
>> }
>> +
>> +
>> +/* Sync the current list to the correct flood port. */
>> +void br_fdb_addrs_sync(struct net_bridge *br)
>> +{
>> + struct net_bridge_port *p;
>> + int err;
>> +
>> + /* This function has to run under RTNL.
>> + * Program the user added addresses into the proper port. This
>> + * fuction follows the following algorithm:
>> + * a) If only 1 port is flooding:
>> + * - write all the addresses to this one port.
>> + */
>> + if (br->n_flood_ports == 1) {
>> + p = br->c_flood_port;
>> + netif_addr_lock(p->dev);
>> + err = __hw_addr_sync(&p->dev->uc, &br->conf_addrs,
>> + p->dev->addr_len);
>> + if (!err)
>> + __dev_set_rx_mode(p->dev);
>> + netif_addr_unlock(p->dev);
>> +
>> + }
>> +}
>> +
>> +void br_fdb_addrs_unsync(struct net_bridge *br)
>> +{
>> + struct net_bridge_port *p = br->c_flood_port;
>> +
>> + if (!p)
>> + return;
>> +
>> + netif_addr_lock(p->dev);
>> + __hw_addr_unsync(&p->dev->uc, &br->conf_addrs, p->dev->addr_len);
>> + __dev_set_rx_mode(p->dev);
>> + netif_addr_unlock(p->dev);
>> +}
>> +
>> +/* When a static FDB entry is added, the mac address from the entry is
>> + * added to the bridge private HW address list and all required ports
>> + * are then updated with the new information.
>> + * Called under RTNL.
>> + */
>> +static int br_addr_add(struct net_bridge *br, const unsigned char *addr)
>> +{
>> + int err;
>> +
>> + ASSERT_RTNL();
>> +
>> + err = __hw_addr_add(&br->conf_addrs, addr, br->dev->addr_len,
>> + NETDEV_HW_ADDR_T_UNICAST);
>> +
>> + if (!err)
>> + br_fdb_addrs_sync(br);
>> +
>> + return err;
>> +}
>> +
>> +/* When a static FDB entry is deleted, the HW address from that entry is
>> + * also removed from the bridge private HW address list and updates all
>> + * the ports with needed information.
>> + * Called under RTNL.
>> + */
>> +static int br_addr_del(struct net_bridge *br, const unsigned char *addr)
>> +{
>> + int err;
>> +
>> + ASSERT_RTNL();
>> +
>> + err = __hw_addr_del(&br->conf_addrs, addr, br->dev->addr_len,
>> + NETDEV_HW_ADDR_T_UNICAST);
>> + if (!err)
>> + br_fdb_addrs_sync(br);
>> +
>> + return err;
>> +}
>> +
>> diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c
>> index f072b34..e782c2e 100644
>> --- a/net/bridge/br_if.c
>> +++ b/net/bridge/br_if.c
>> @@ -144,7 +144,7 @@ static void del_nbp(struct net_bridge_port *p)
>>
>> br_ifinfo_notify(RTM_DELLINK, p);
>>
>> - list_del_rcu(&p->list);
>> + dev->priv_flags &= ~IFF_BRIDGE_PORT;
>>
>> if (p->flags & BR_FLOOD)
>> br_del_flood_port(p, br);
>> @@ -152,7 +152,7 @@ static void del_nbp(struct net_bridge_port *p)
>> nbp_vlan_flush(p);
>> br_fdb_delete_by_port(br, p, 1);
>>
>> - dev->priv_flags &= ~IFF_BRIDGE_PORT;
>> + list_del_rcu(&p->list);
>>
>> netdev_rx_handler_unregister(dev);
>>
>> @@ -473,6 +473,8 @@ static void br_add_flood_port(struct net_bridge_port *p, struct net_bridge *br)
>> br->n_flood_ports++;
>> if (br->n_flood_ports == 1)
>> br->c_flood_port = p;
>> +
>> + br_fdb_addrs_sync(br);
>> }
>>
>> static void br_del_flood_port(struct net_bridge_port *p, struct net_bridge *br)
>> @@ -485,13 +487,17 @@ static void br_del_flood_port(struct net_bridge_port *p, struct net_bridge *br)
>> * set it if it is not set.
>> */
>> br->n_flood_ports--;
>> - if (p == br->c_flood_port)
>> + if (p == br->c_flood_port) {
>> + br_fdb_addrs_unsync(br);
>> br->c_flood_port = NULL;
>> + }
>>
>> if (br->n_flood_ports == 1) {
>> list_for_each_entry(port, &p->br->port_list, list) {
>> - if (port->flags & BR_FLOOD) {
>> + if (br_port_exists(port->dev) &&
>> + (port->flags & BR_FLOOD)) {
>> br->c_flood_port = port;
>> + br_fdb_addrs_sync(br);
>> break;
>> }
>> }
>> diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
>> index 26a3987..40a6927 100644
>> --- a/net/bridge/br_private.h
>> +++ b/net/bridge/br_private.h
>> @@ -296,6 +296,7 @@ struct net_bridge
>> u8 vlan_enabled;
>> struct net_port_vlans __rcu *vlan_info;
>> #endif
>> + struct netdev_hw_addr_list conf_addrs;
>> };
>>
>> struct br_input_skb_cb {
>> @@ -397,6 +398,8 @@ int br_fdb_add(struct ndmsg *nlh, struct nlattr *tb[], struct net_device *dev,
>> const unsigned char *addr, u16 nlh_flags);
>> int br_fdb_dump(struct sk_buff *skb, struct netlink_callback *cb,
>> struct net_device *dev, int idx);
>> +void br_fdb_addrs_sync(struct net_bridge *br);
>> +void br_fdb_addrs_unsync(struct net_bridge *br);
>>
>> /* br_forward.c */
>> void br_deliver(const struct net_bridge_port *to, struct sk_buff *skb);
>> diff --git a/net/core/dev.c b/net/core/dev.c
>> index 4ad1b78..eca4d476 100644
>> --- a/net/core/dev.c
>> +++ b/net/core/dev.c
>> @@ -5212,6 +5212,7 @@ void __dev_set_rx_mode(struct net_device *dev)
>> if (ops->ndo_set_rx_mode)
>> ops->ndo_set_rx_mode(dev);
>> }
>> +EXPORT_SYMBOL(__dev_set_rx_mode);
>>
>> void dev_set_rx_mode(struct net_device *dev)
>> {
>> diff --git a/net/core/dev_addr_lists.c b/net/core/dev_addr_lists.c
>> index 329d579..3de44a3 100644
>> --- a/net/core/dev_addr_lists.c
>> +++ b/net/core/dev_addr_lists.c
>> @@ -81,13 +81,14 @@ static int __hw_addr_add_ex(struct netdev_hw_addr_list *list,
>> sync);
>> }
>>
>> -static int __hw_addr_add(struct netdev_hw_addr_list *list,
>> - const unsigned char *addr, int addr_len,
>> - unsigned char addr_type)
>> +int __hw_addr_add(struct netdev_hw_addr_list *list,
>> + const unsigned char *addr, int addr_len,
>> + unsigned char addr_type)
>> {
>> return __hw_addr_add_ex(list, addr, addr_len, addr_type, false, false,
>> 0);
>> }
>> +EXPORT_SYMBOL(__hw_addr_add);
>>
>> static int __hw_addr_del_entry(struct netdev_hw_addr_list *list,
>> struct netdev_hw_addr *ha, bool global,
>> @@ -127,12 +128,13 @@ static int __hw_addr_del_ex(struct netdev_hw_addr_list *list,
>> return -ENOENT;
>> }
>>
>> -static int __hw_addr_del(struct netdev_hw_addr_list *list,
>> - const unsigned char *addr, int addr_len,
>> - unsigned char addr_type)
>> +int __hw_addr_del(struct netdev_hw_addr_list *list,
>> + const unsigned char *addr, int addr_len,
>> + unsigned char addr_type)
>> {
>> return __hw_addr_del_ex(list, addr, addr_len, addr_type, false, false);
>> }
>> +EXPORT_SYMBOL(__hw_addr_del);
>>
>> static int __hw_addr_sync_one(struct netdev_hw_addr_list *to_list,
>> struct netdev_hw_addr *ha,
>> --
>> 1.8.5.3
--
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
Powered by blists - more mailing lists