[PATCH 1/2] net: bonding: fix type-confusion in bonding header_ops

Kota Toda posted 2 patches 1 month ago
There is a newer version of this series
[PATCH 1/2] net: bonding: fix type-confusion in bonding header_ops
Posted by Kota Toda 1 month ago
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.

Signed-off-by: Kota Toda <kota.toda@gmo-cybersecurity.com>
Co-developed-by: Yuki Koike <yuki.koike@gmo-cybersecurity.com>
Signed-off-by: Yuki Koike <yuki.koike@gmo-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..2efad75ac 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;
+	cache_update = READ_ONCE(slave_dev->header_ops->cache_update);
+	if (!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);
+	}
+
+	WRITE_ONCE(bond_dev->header_ops, &bond->bond_header_ops);
+	bond->header_slave_dev      = slave_dev;

 	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) {
+		WRITE_ONCE(bond_dev->header_ops, NULL);
+		bond->header_slave_dev = 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..cf8206187 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
Re: [PATCH 1/2] net: bonding: fix type-confusion in bonding header_ops
Posted by Eric Dumazet 1 month ago
On Thu, Mar 5, 2026 at 10:27 AM Kota Toda
<kota.toda@gmo-cybersecurity.com> wrote:
>
> 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.

I suggest you write a detailed changelog.
Perhaps get help from various LLM.
Do not assume we have the context now.
We receive hundreds of patches per week, do not assume we have
infinite time for each of them.
Help us.
Please ?
Thank you.
Re: [PATCH 1/2] net: bonding: fix type-confusion in bonding header_ops
Posted by 戸田晃太 1 month ago
Thank you for the suggestion and for taking the time to review this.

I have sent the patch series as [PATCH v4 2/2] net: bonding: fix
type-confusion in bonding header_ops, and I expanded the changelog in
patch 1/2 as well (not only in the cover letter).

I would appreciate it if you could take a look.

2026年3月5日(木) 18:32 Eric Dumazet <edumazet@google.com>:


>
> On Thu, Mar 5, 2026 at 10:27 AM Kota Toda
> <kota.toda@gmo-cybersecurity.com> wrote:
> >
> > 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.
>
> I suggest you write a detailed changelog.
> Perhaps get help from various LLM.
> Do not assume we have the context now.
> We receive hundreds of patches per week, do not assume we have
> infinite time for each of them.
> Help us.
> Please ?
> Thank you.