| 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: <CAA3_Gno0c6uGwuxBcsDgZt2a_VriABPek31-1w-R8c9CdJw5Ow@mail.gmail.com>
Date: Thu, 5 Feb 2026 19:57:00 +0900
From: 戸田晃太 <kota.toda@...-cybersecurity.com>
To: Eric Dumazet <edumazet@...gle.com>
Cc: pabeni@...hat.com, netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
小池悠生 <yuki.koike@...-cybersecurity.com>,
戸田晃太 <kota.toda@...-cybersecurity.com>
Subject: [PATCH net 1/2] net: bonding: fix type-confusion in bonding header_ops
Add two members to struct bonding, bond_header_ops and
header_slave_dev, to avoid type-confusion while keeping track of the
slave's header_ops.
Fixes: 1284cd3a2b740 (bonding: two small fixes for IPoIB support)
Signed-off-by: Kota Toda <kota.toda@...-cybersecurity.com>
Signed-off-by: Yuki Koike <yuki.koike@...-cybersecurity.com>
Co-developed-by: Yuki Koike <yuki.koike@...-cybersecurity.com>
Reviewed-by: Paolo Abeni <pabeni@...hat.com>
Reported-by: Kota Toda <kota.toda@...-cybersecurity.com>
---
drivers/net/bonding/bond_main.c | 66 ++++++++++++++++++++++++++++++++-
include/net/bonding.h | 5 +++
2 files changed, 70 insertions(+), 1 deletion(-)
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index f17a170d1..5ecc64e38 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -1616,14 +1616,70 @@ static void bond_compute_features(struct bonding *bond)
netdev_change_features(bond_dev);
}
+static int bond_hard_header(struct sk_buff *skb, struct net_device *dev,
+ unsigned short type, const void *daddr,
+ const void *saddr, unsigned int len)
+{
+ struct bonding *bond = netdev_priv(dev);
+ struct net_device *slave_dev;
+
+ slave_dev = bond->header_slave_dev;
+
+ return dev_hard_header(skb, slave_dev, type, daddr, saddr, len);
+}
+
+static void bond_header_cache_update(struct hh_cache *hh, const
+struct net_device *dev,
+ const unsigned char *haddr)
+{
+ const struct bonding *bond = netdev_priv(dev);
+ void (*cache_update)(struct hh_cache *hh,
+ const struct net_device *dev,
+ const unsigned char *haddr);
+ struct net_device *slave_dev;
+
+ slave_dev = bond->header_slave_dev;
+
+ if (!slave_dev->header_ops || !(cache_update =
READ_ONCE(slave_dev->header_ops->cache_update)))
+ return;
+
+ cache_update(hh, slave_dev, haddr);
+}
+
+
static void bond_setup_by_slave(struct net_device *bond_dev,
struct net_device *slave_dev)
{
+ struct bonding *bond = netdev_priv(bond_dev);
bool was_up = !!(bond_dev->flags & IFF_UP);
dev_close(bond_dev);
- bond_dev->header_ops = slave_dev->header_ops;
+ /* Some functions are given dev as an argument
+ * while others not. When dev is not given, we cannot
+ * find out what is the slave device through struct bonding
+ * (the private data of bond_dev). Therefore, we need a raw
+ * header_ops variable instead of its pointer to const header_ops
+ * and assign slave's functions directly.
+ * For the other case, we set the wrapper functions that pass
+ * slave_dev to the wrapped functions.
+ */
+ bond->bond_header_ops.create = bond_hard_header;
+ bond->bond_header_ops.cache_update = bond_header_cache_update;
+ if (slave_dev->header_ops) {
+ WRITE_ONCE(bond->bond_header_ops.parse, slave_dev->header_ops->parse);
+ WRITE_ONCE(bond->bond_header_ops.cache, slave_dev->header_ops->cache);
+ WRITE_ONCE(bond->bond_header_ops.validate,
slave_dev->header_ops->validate);
+ WRITE_ONCE(bond->bond_header_ops.parse_protocol,
slave_dev->header_ops->parse_protocol);
+ } else {
+ WRITE_ONCE(bond->bond_header_ops.parse, NULL);
+ WRITE_ONCE(bond->bond_header_ops.cache, NULL);
+ WRITE_ONCE(bond->bond_header_ops.validate, NULL);
+ WRITE_ONCE(bond->bond_header_ops.parse_protocol, NULL);
+ }
+
+ bond->header_slave_dev = slave_dev;
+ bond_dev->header_ops = &bond->bond_header_ops;
bond_dev->type = slave_dev->type;
bond_dev->hard_header_len = slave_dev->hard_header_len;
@@ -2682,6 +2738,14 @@ static int bond_release_and_destroy(struct
net_device *bond_dev,
struct bonding *bond = netdev_priv(bond_dev);
int ret;
+ /* If slave_dev is the earliest registered one, we must clear
+ * the variables related to header_ops to avoid dangling pointer.
+ */
+ if (bond->header_slave_dev == slave_dev) {
+ bond->header_slave_dev = NULL;
+ bond_dev->header_ops = NULL;
+ }
+
ret = __bond_release_one(bond_dev, slave_dev, false, true);
if (ret == 0 && !bond_has_slaves(bond) &&
bond_dev->reg_state != NETREG_UNREGISTERING) {
diff --git a/include/net/bonding.h b/include/net/bonding.h
index 95f67b308..c37800e17 100644
--- a/include/net/bonding.h
+++ b/include/net/bonding.h
@@ -215,6 +215,11 @@ struct bond_ipsec {
*/
struct bonding {
struct net_device *dev; /* first - useful for panic debug */
+ struct net_device *header_slave_dev; /* slave net_device for
header_ops */
+ /* maintained as a non-const variable
+ * because bond's header_ops should change depending on slaves.
+ */
+ struct header_ops bond_header_ops;
struct slave __rcu *curr_active_slave;
struct slave __rcu *current_arp_slave;
struct slave __rcu *primary_slave;
--
2.53.0
2026年2月3日(火) 2:11 Eric Dumazet <edumazet@...gle.com>:
>
> On Wed, Jan 28, 2026 at 11:46 AM 戸田晃太 <kota.toda@...-cybersecurity.com> wrote:
> >
> > Here is the patch with the barriers added, based on v6.12.67.
> >
> > However, as Yuki said, we are wondering if this would be considered an
> > acceptable change
> > from the perspective of the maintainers (or in terms of Linux kernel
> > culture). This is because
> > the patch adds `READ_ONCE` to several locations outside of bonding subsystem.
> > Please let me know if you have any concerns regarding this point.
> >
> > > Also, please clarify what happens with stacks of two or more bonding devices ?
> >
> > To clarify, currently the `header_ops` of the bottom-most
> > interface are used regardless of the number of bonding layers.
> > This patch changes it so that `&bond->bond_header_ops` is used
> > as the bond device's `header_ops`, regardless of the stack depth.
>
> Could you try to cook a patch series perhaps ?
>
> The READ_ONCE()/WRITE_ONCE() on dev->header_ops->cache could be done separately.
>
> Thanks.
Powered by blists - more mailing lists