| 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: <58777.1729526711@vermin>
Date: Mon, 21 Oct 2024 18:05:11 +0200
From: Jay Vosburgh <jv@...sburgh.net>
To: Hangbin Liu <liuhangbin@...il.com>
cc: netdev@...r.kernel.org, Andy Gospodarek <andy@...yhouse.net>,
"David S. Miller" <davem@...emloft.net>,
Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>,
Paolo Abeni <pabeni@...hat.com>,
Nikolay Aleksandrov <razor@...ckwall.org>,
Simon Horman <horms@...nel.org>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH net] bonding: add ns target multicast address to slave device
Hangbin Liu <liuhangbin@...il.com> wrote:
>Commit 4598380f9c54 ("bonding: fix ns validation on backup slaves")
>tried to resolve the issue where backup slaves couldn't be brought up when
>receiving IPv6 Neighbor Solicitation (NS) messages. However, this fix only
>worked for drivers that receive all multicast messages, such as the veth
>interface.
>
>For standard drivers, the NS multicast message is silently dropped because
>the slave device is not a member of the NS target multicast group.
>
>To address this, we need to make the slave device join the NS target
>multicast group, ensuring it can receive these IPv6 NS messages to validate
>the slave’s status properly.
>
>Fixes: 4e24be018eb9 ("bonding: add new parameter ns_targets")
>Signed-off-by: Hangbin Liu <liuhangbin@...il.com>
This seems fairly involved; would it be simpler to have
bond_hw_addr_swap() and/or bond_change_active_slave() insure that the
MAC multicast list is configured in the backup interface if arp_validate
is set appropriately and there's a NS target configured? That will make
the MAC multicast list more inclusive than necessary, but I think the
implementation will be much less involved.
-J
>---
>Another way is to set IFF_ALLMULTI flag for slaves. But I think that
>would affect too much.
>---
> drivers/net/bonding/bond_main.c | 11 ++++++++
> drivers/net/bonding/bond_options.c | 44 +++++++++++++++++++++++++++++-
> include/net/bond_options.h | 2 ++
> 3 files changed, 56 insertions(+), 1 deletion(-)
>
>diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
>index b1bffd8e9a95..04ccbd41fb0c 100644
>--- a/drivers/net/bonding/bond_main.c
>+++ b/drivers/net/bonding/bond_main.c
>@@ -2350,6 +2350,11 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev,
> if (bond_mode_can_use_xmit_hash(bond))
> bond_update_slave_arr(bond, NULL);
>
>+#if IS_ENABLED(CONFIG_IPV6)
>+ if (slave_dev->flags & IFF_MULTICAST)
>+ /* set target NS maddrs for new slave */
>+ slave_set_ns_maddr(bond, slave_dev, true);
>+#endif
>
> if (!slave_dev->netdev_ops->ndo_bpf ||
> !slave_dev->netdev_ops->ndo_xdp_xmit) {
>@@ -2503,6 +2508,12 @@ static int __bond_release_one(struct net_device *bond_dev,
> /* recompute stats just before removing the slave */
> bond_get_stats(bond->dev, &bond->bond_stats);
>
>+#if IS_ENABLED(CONFIG_IPV6)
>+ if (slave_dev->flags & IFF_MULTICAST)
>+ /* clear all target NS maddrs */
>+ slave_set_ns_maddr(bond, slave_dev, false);
>+#endif
>+
> if (bond->xdp_prog) {
> struct netdev_bpf xdp = {
> .command = XDP_SETUP_PROG,
>diff --git a/drivers/net/bonding/bond_options.c b/drivers/net/bonding/bond_options.c
>index 95d59a18c022..823cb93d2853 100644
>--- a/drivers/net/bonding/bond_options.c
>+++ b/drivers/net/bonding/bond_options.c
>@@ -1234,17 +1234,41 @@ static int bond_option_arp_ip_targets_set(struct bonding *bond,
> }
>
> #if IS_ENABLED(CONFIG_IPV6)
>+/* convert IPv6 address to link-local solicited-node multicast mac address */
>+static void ipv6_addr_to_solicited_mac(const struct in6_addr *addr,
>+ unsigned char mac[ETH_ALEN])
>+{
>+ mac[0] = 0x33;
>+ mac[1] = 0x33;
>+ mac[2] = 0xFF;
>+ mac[3] = addr->s6_addr[13];
>+ mac[4] = addr->s6_addr[14];
>+ mac[5] = addr->s6_addr[15];
>+}
>+
> static void _bond_options_ns_ip6_target_set(struct bonding *bond, int slot,
> struct in6_addr *target,
> unsigned long last_rx)
> {
>+ unsigned char target_maddr[ETH_ALEN], slot_maddr[ETH_ALEN];
> struct in6_addr *targets = bond->params.ns_targets;
> struct list_head *iter;
> struct slave *slave;
>
>+ if (!ipv6_addr_any(target))
>+ ipv6_addr_to_solicited_mac(target, target_maddr);
> if (slot >= 0 && slot < BOND_MAX_NS_TARGETS) {
>- bond_for_each_slave(bond, slave, iter)
>+ if (!ipv6_addr_any(&targets[slot]))
>+ ipv6_addr_to_solicited_mac(&targets[slot], slot_maddr);
>+ bond_for_each_slave(bond, slave, iter) {
> slave->target_last_arp_rx[slot] = last_rx;
>+ /* remove the previous maddr on salve */
>+ if (!ipv6_addr_any(&targets[slot]))
>+ dev_mc_del(slave->dev, slot_maddr);
>+ /* add new maddr on slave if target is set */
>+ if (!ipv6_addr_any(target))
>+ dev_mc_add(slave->dev, target_maddr);
>+ }
> targets[slot] = *target;
> }
> }
>@@ -1290,6 +1314,24 @@ static int bond_option_ns_ip6_targets_set(struct bonding *bond,
>
> return 0;
> }
>+
>+void slave_set_ns_maddr(struct bonding *bond, struct net_device *slave_dev,
>+ bool add)
>+{
>+ struct in6_addr *targets = bond->params.ns_targets;
>+ unsigned char slot_maddr[ETH_ALEN];
>+ int i;
>+
>+ for (i = 0; i < BOND_MAX_NS_TARGETS; i++) {
>+ if (!ipv6_addr_any(&targets[i])) {
>+ ipv6_addr_to_solicited_mac(&targets[i], slot_maddr);
>+ if (add)
>+ dev_mc_add(slave_dev, slot_maddr);
>+ else
>+ dev_mc_del(slave_dev, slot_maddr);
>+ }
>+ }
>+}
> #else
> static int bond_option_ns_ip6_targets_set(struct bonding *bond,
> const struct bond_opt_value *newval)
>diff --git a/include/net/bond_options.h b/include/net/bond_options.h
>index 473a0147769e..c6c5c1333f37 100644
>--- a/include/net/bond_options.h
>+++ b/include/net/bond_options.h
>@@ -160,6 +160,8 @@ static inline void __bond_opt_init(struct bond_opt_value *optval,
> void bond_option_arp_ip_targets_clear(struct bonding *bond);
> #if IS_ENABLED(CONFIG_IPV6)
> void bond_option_ns_ip6_targets_clear(struct bonding *bond);
>+void slave_set_ns_maddr(struct bonding *bond, struct net_device *slave_dev,
>+ bool add);
> #endif
>
> #endif /* _NET_BOND_OPTIONS_H */
>--
>2.46.0
>
---
-Jay Vosburgh, jv@...sburgh.net
Powered by blists - more mailing lists