public inbox for netdev@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4 0/2] net: bonding: fix type-confusion in bonding header_ops
@ 2026-03-05 11:07 Kota Toda
  2026-03-05 11:07 ` [PATCH v4 1/2] " Kota Toda
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Kota Toda @ 2026-03-05 11:07 UTC (permalink / raw)
  To: Jeff Garzik, Jay Vosburgh; +Cc: Kota Toda, netdev, linux-kernel, Yuki Koike

In bond_setup_by_slave(), the slave’s header_ops are unconditionally
copied into the bonding device. As a result, the bonding device may invoke
the slave-specific header operations on itself, causing
netdev_priv(bond_dev) (a struct bonding) to be incorrectly interpreted
as the slave's private-data type.

This type-confusion bug can lead to out-of-bounds writes into the skb,
resulting in memory corruption.

Patch 1 stores the slave's header_ops in struct bonding and sets
wrapper callbacks in bond_In bond_setup_by_slave(), the slave’s
header_ops are unconditionally
copied into the bonding device. As a result, the bonding device may invoke
the slave-specific header operations on itself, causing
netdev_priv(bond_dev) (a struct bonding) to be incorrectly interpreted
as the slave's private-data type.

Patch 2 uses READ_ONCE when loading header_ops callbacks
to avoid races with concurrent updates.

Fixes: 1284cd3a2b74 ("bonding: two small fixes for IPoIB support")
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>

Kota Toda (2):
  net: bonding: fix type-confusion in bonding header_ops
  net: add READ_ONCE for header_ops callbacks

 drivers/net/bonding/bond_main.c | 67 ++++++++++++++++++++++++++++++++-
 include/linux/netdevice.h       | 41 ++++++++++++++------
 include/net/bonding.h           |  5 +++
 include/net/cfg802154.h         |  2 +-
 net/core/neighbour.c            |  6 +--
 net/ipv4/arp.c                  |  2 +-
 net/ipv6/ndisc.c                |  2 +-
 7 files changed, 106 insertions(+), 19 deletions(-)

-- 
2.53.0


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH v4 1/2] net: bonding: fix type-confusion in bonding header_ops
  2026-03-05 11:07 [PATCH v4 0/2] net: bonding: fix type-confusion in bonding header_ops Kota Toda
@ 2026-03-05 11:07 ` Kota Toda
  2026-03-05 11:07 ` [PATCH v4 2/2] net: read header_ops callbacks with READ_ONCE() Kota Toda
  2026-03-05 20:59 ` [PATCH v4 0/2] net: bonding: fix type-confusion in bonding header_ops Jay Vosburgh
  2 siblings, 0 replies; 7+ messages in thread
From: Kota Toda @ 2026-03-05 11:07 UTC (permalink / raw)
  To: Jeff Garzik, Jay Vosburgh, Jay Vosburgh, Andy Gospodarek,
	Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Simon Horman
  Cc: Kota Toda, netdev, linux-kernel, Yuki Koike

In bond_setup_by_slave(), the slave’s header_ops are unconditionally
copied into the bonding device. As a result, the bonding device may invoke
the slave-specific header operations on itself, causing
netdev_priv(bond_dev) (a struct bonding) to be incorrectly interpreted
as the slave's private-data type.

This type-confusion bug can lead to out-of-bounds writes into the skb,
resulting in memory corruption.

This patch stores the slave's header_ops in struct bonding and sets
wrapper callbacks in bond_In bond_setup_by_slave(), the slave’s
header_ops are unconditionally
copied into the bonding device. As a result, the bonding device may invoke
the slave-specific header operations on itself, causing
netdev_priv(bond_dev) (a struct bonding) to be incorrectly interpreted
as the slave's private-data type.

Fixes: 1284cd3a2b74 ("bonding: two small fixes for IPoIB support")
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 | 67 ++++++++++++++++++++++++++++++++-
 include/net/bonding.h           |  5 +++
 2 files changed, 71 insertions(+), 1 deletion(-)

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index f17a170d1..14d3e5298 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -1616,14 +1616,71 @@ 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)
+		return;
+	cache_update = READ_ONCE(slave_dev->header_ops->cache_update);
+	if (!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 +2739,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


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v4 2/2] net: read header_ops callbacks with READ_ONCE()
  2026-03-05 11:07 [PATCH v4 0/2] net: bonding: fix type-confusion in bonding header_ops Kota Toda
  2026-03-05 11:07 ` [PATCH v4 1/2] " Kota Toda
@ 2026-03-05 11:07 ` Kota Toda
  2026-03-05 20:59 ` [PATCH v4 0/2] net: bonding: fix type-confusion in bonding header_ops Jay Vosburgh
  2 siblings, 0 replies; 7+ messages in thread
From: Kota Toda @ 2026-03-05 11:07 UTC (permalink / raw)
  To: Jeff Garzik, Jay Vosburgh, Andrew Lunn, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman,
	Alexander Aring, Stefan Schmidt, Miquel Raynal, David Ahern
  Cc: Kota Toda, netdev, linux-kernel, Yuki Koike, linux-wpan

Bonding now updates its header_ops callbacks at runtime, so lockless
readers can observe concurrent callback updates.

This patch loads header_ops callbacks with READ_ONCE() and 
call the loaded function pointer, instead of 
re-reading through dev->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>
---
 include/linux/netdevice.h | 41 +++++++++++++++++++++++++++------------
 include/net/cfg802154.h   |  2 +-
 net/core/neighbour.c      |  6 +++---
 net/ipv4/arp.c            |  2 +-
 net/ipv6/ndisc.c          |  2 +-
 5 files changed, 35 insertions(+), 18 deletions(-)

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 77a99c8ab..79fb0864a 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -3150,35 +3150,50 @@ static inline int dev_hard_header(struct sk_buff *skb, struct net_device *dev,
 				  const void *daddr, const void *saddr,
 				  unsigned int len)
 {
-	if (!dev->header_ops || !dev->header_ops->create)
-		return 0;
+	int (*create)(struct sk_buff *skb, struct net_device *dev,
+		      unsigned short type, const void *daddr,
+		      const void *saddr, unsigned int len);
 
-	return dev->header_ops->create(skb, dev, type, daddr, saddr, len);
+	if (!dev->header_ops)
+		return 0;
+	create = READ_ONCE(dev->header_ops->create);
+	if (!create)
+		return 0;
+	return create(skb, dev, type, daddr, saddr, len);
 }
 
 static inline int dev_parse_header(const struct sk_buff *skb,
 				   unsigned char *haddr)
 {
+	int (*parse)(const struct sk_buff *skb, unsigned char *haddr);
 	const struct net_device *dev = skb->dev;
 
-	if (!dev->header_ops || !dev->header_ops->parse)
+	if (!dev->header_ops)
 		return 0;
-	return dev->header_ops->parse(skb, haddr);
+	parse = READ_ONCE(dev->header_ops->parse);
+	if (!parse)
+		return 0;
+	return parse(skb, haddr);
 }
 
 static inline __be16 dev_parse_header_protocol(const struct sk_buff *skb)
 {
+	__be16	(*parse_protocol)(const struct sk_buff *skb);
 	const struct net_device *dev = skb->dev;
 
-	if (!dev->header_ops || !dev->header_ops->parse_protocol)
+	if (!dev->header_ops)
+		return 0;
+	parse_protocol = READ_ONCE(dev->header_ops->parse_protocol);
+	if (!parse_protocol)
 		return 0;
-	return dev->header_ops->parse_protocol(skb);
+	return parse_protocol(skb);
 }
 
 /* ll_header must have at least hard_header_len allocated */
 static inline bool dev_validate_header(const struct net_device *dev,
 				       char *ll_header, int len)
 {
+	bool	(*validate)(const char *ll_header, unsigned int len);
 	if (likely(len >= dev->hard_header_len))
 		return true;
 	if (len < dev->min_header_len)
@@ -3189,15 +3204,17 @@ static inline bool dev_validate_header(const struct net_device *dev,
 		return true;
 	}
 
-	if (dev->header_ops && dev->header_ops->validate)
-		return dev->header_ops->validate(ll_header, len);
-
-	return false;
+	if (!dev->header_ops)
+		return false;
+	validate = READ_ONCE(dev->header_ops->validate);
+	if (!validate)
+		return false;
+	return validate(ll_header, len);
 }
 
 static inline bool dev_has_header(const struct net_device *dev)
 {
-	return dev->header_ops && dev->header_ops->create;
+	return dev->header_ops && READ_ONCE(dev->header_ops->create);
 }
 
 /*
diff --git a/include/net/cfg802154.h b/include/net/cfg802154.h
index 76d2cd2e2..dec638763 100644
--- a/include/net/cfg802154.h
+++ b/include/net/cfg802154.h
@@ -522,7 +522,7 @@ wpan_dev_hard_header(struct sk_buff *skb, struct net_device *dev,
 {
 	struct wpan_dev *wpan_dev = dev->ieee802154_ptr;
 
-	return wpan_dev->header_ops->create(skb, dev, daddr, saddr, len);
+	return READ_ONCE(wpan_dev->header_ops->create)(skb, dev, daddr, saddr, len);
 }
 #endif
 
diff --git a/net/core/neighbour.c b/net/core/neighbour.c
index 96786016d..ff948e35e 100644
--- a/net/core/neighbour.c
+++ b/net/core/neighbour.c
@@ -1270,7 +1270,7 @@ static void neigh_update_hhs(struct neighbour *neigh)
 		= NULL;
 
 	if (neigh->dev->header_ops)
-		update = neigh->dev->header_ops->cache_update;
+		update = READ_ONCE(neigh->dev->header_ops->cache_update);
 
 	if (update) {
 		hh = &neigh->hh;
@@ -1540,7 +1540,7 @@ static void neigh_hh_init(struct neighbour *n)
 	 * hh_cache entry.
 	 */
 	if (!hh->hh_len)
-		dev->header_ops->cache(n, hh, prot);
+		READ_ONCE(dev->header_ops->cache)(n, hh, prot);
 
 	write_unlock_bh(&n->lock);
 }
@@ -1556,7 +1556,7 @@ int neigh_resolve_output(struct neighbour *neigh, struct sk_buff *skb)
 		struct net_device *dev = neigh->dev;
 		unsigned int seq;
 
-		if (dev->header_ops->cache && !READ_ONCE(neigh->hh.hh_len))
+		if (READ_ONCE(dev->header_ops->cache) && !READ_ONCE(neigh->hh.hh_len))
 			neigh_hh_init(neigh);
 
 		do {
diff --git a/net/ipv4/arp.c b/net/ipv4/arp.c
index 7822b2144..421bea6eb 100644
--- a/net/ipv4/arp.c
+++ b/net/ipv4/arp.c
@@ -278,7 +278,7 @@ static int arp_constructor(struct neighbour *neigh)
 			memcpy(neigh->ha, dev->broadcast, dev->addr_len);
 		}
 
-		if (dev->header_ops->cache)
+		if (READ_ONCE(dev->header_ops->cache))
 			neigh->ops = &arp_hh_ops;
 		else
 			neigh->ops = &arp_generic_ops;
diff --git a/net/ipv6/ndisc.c b/net/ipv6/ndisc.c
index d961e6c2d..d81f509ec 100644
--- a/net/ipv6/ndisc.c
+++ b/net/ipv6/ndisc.c
@@ -361,7 +361,7 @@ static int ndisc_constructor(struct neighbour *neigh)
 			neigh->nud_state = NUD_NOARP;
 			memcpy(neigh->ha, dev->broadcast, dev->addr_len);
 		}
-		if (dev->header_ops->cache)
+		if (READ_ONCE(dev->header_ops->cache))
 			neigh->ops = &ndisc_hh_ops;
 		else
 			neigh->ops = &ndisc_generic_ops;
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH v4 0/2] net: bonding: fix type-confusion in bonding header_ops
  2026-03-05 11:07 [PATCH v4 0/2] net: bonding: fix type-confusion in bonding header_ops Kota Toda
  2026-03-05 11:07 ` [PATCH v4 1/2] " Kota Toda
  2026-03-05 11:07 ` [PATCH v4 2/2] net: read header_ops callbacks with READ_ONCE() Kota Toda
@ 2026-03-05 20:59 ` Jay Vosburgh
  2026-03-06  8:06   ` Jiayuan Chen
                     ` (2 more replies)
  2 siblings, 3 replies; 7+ messages in thread
From: Jay Vosburgh @ 2026-03-05 20:59 UTC (permalink / raw)
  To: Kota Toda; +Cc: Jeff Garzik, netdev, linux-kernel, Yuki Koike, Jiayuan Chen

Kota Toda <kota.toda@gmo-cybersecurity.com> wrote:

>In bond_setup_by_slave(), the slave’s header_ops are unconditionally
>copied into the bonding device. As a result, the bonding device may invoke
>the slave-specific header operations on itself, causing
>netdev_priv(bond_dev) (a struct bonding) to be incorrectly interpreted
>as the slave's private-data type.
>
>This type-confusion bug can lead to out-of-bounds writes into the skb,
>resulting in memory corruption.

	A few days ago, Jiayuan Chen <jiayuan.chen@linux.dev> posted a
fix for what sounds like the same problem[0].  Their solution appears to
be much less complicated.

	I also wonder how this bug was discovered.  The code in question
hasn't changed in many years, and now there are two independent fixes
within a week.

[0] https://lore.kernel.org/netdev/20260228095854.391093-1-jiayuan.chen@linux.dev/

>Patch 1 stores the slave's header_ops in struct bonding and sets
>wrapper callbacks in bond_In bond_setup_by_slave(), the slave’s
>header_ops are unconditionally
>copied into the bonding device. As a result, the bonding device may invoke
>the slave-specific header operations on itself, causing
>netdev_priv(bond_dev) (a struct bonding) to be incorrectly interpreted
>as the slave's private-data type.
>
>Patch 2 uses READ_ONCE when loading header_ops callbacks
>to avoid races with concurrent updates.

	With the READ_ONCE changes in a separate patch, does that mean
that patch 1 by itself is subject to race conditions that would result
in errors?  If so, that's not acceptable, every patch must stand alone
and not break the kernel.

	-J

>Fixes: 1284cd3a2b74 ("bonding: two small fixes for IPoIB support")
>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>
>
>Kota Toda (2):
>  net: bonding: fix type-confusion in bonding header_ops
>  net: add READ_ONCE for header_ops callbacks
>
> drivers/net/bonding/bond_main.c | 67 ++++++++++++++++++++++++++++++++-
> include/linux/netdevice.h       | 41 ++++++++++++++------
> include/net/bonding.h           |  5 +++
> include/net/cfg802154.h         |  2 +-
> net/core/neighbour.c            |  6 +--
> net/ipv4/arp.c                  |  2 +-
> net/ipv6/ndisc.c                |  2 +-
> 7 files changed, 106 insertions(+), 19 deletions(-)
>
>-- 
>2.53.0
>
>

---
	-Jay Vosburgh, jv@jvosburgh.net


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v4 0/2] net: bonding: fix type-confusion in bonding header_ops
  2026-03-05 20:59 ` [PATCH v4 0/2] net: bonding: fix type-confusion in bonding header_ops Jay Vosburgh
@ 2026-03-06  8:06   ` Jiayuan Chen
  2026-03-06  8:13   ` Jiayuan Chen
  2026-03-10 10:45   ` 戸田晃太
  2 siblings, 0 replies; 7+ messages in thread
From: Jiayuan Chen @ 2026-03-06  8:06 UTC (permalink / raw)
  To: Jay Vosburgh; +Cc: Jeff Garzik, netdev, linux-kernel, Yuki Koike

March 6, 2026 at 04:59, "Jay Vosburgh" <jv@jvosburgh.net mailto:jv@jvosburgh.net?to=%22Jay%20Vosburgh%22%20%3Cjv%40jvosburgh.net%3E > wrote:


> 
> Kota Toda <kota.toda@gmo-cybersecurity.com> wrote:
> 
> > 
> > In bond_setup_by_slave(), the slave’s header_ops are unconditionally
> > copied into the bonding device. As a result, the bonding device may invoke
> > the slave-specific header operations on itself, causing
> > netdev_priv(bond_dev) (a struct bonding) to be incorrectly interpreted
> > as the slave's private-data type.
> > 
> > This type-confusion bug can lead to out-of-bounds writes into the skb,
> > resulting in memory corruption.
> > 
>  A few days ago, Jiayuan Chen <jiayuan.chen@linux.dev> posted a
> fix for what sounds like the same problem[0]. Their solution appears to
> be much less complicated.
> 
>  I also wonder how this bug was discovered. The code in question
> hasn't changed in many years, and now there are two independent fixes
> within a week.

This issue has existed for years, but was likely masked by other bugs in the IP GRE
module.
https://syzkaller.appspot.com/bug?extid=4c63f36709a642f801c5
https://syzkaller.appspot.com/bug?id=77135d6c2fc52eff1b3c561912fbec39761e0461

The recent commit e67c577d8989 ("ipv4: ip_gre: make ipgre_header() robust"), which
introduced pskb_expand_head, has made the wrong type-casting issue more apparent.

v2 was sent:
https://lore.kernel.org/netdev/20260306021508.222062-1-jiayuan.chen@linux.dev/


> [0] https://lore.kernel.org/netdev/20260228095854.391093-1-jiayuan.chen@linux.dev/
> 
> > 
> > Patch 1 stores the slave's header_ops in struct bonding and sets
> > wrapper callbacks in bond_In bond_setup_by_slave(), the slave’s
> > header_ops are unconditionally
> > copied into the bonding device. As a result, the bonding device may invoke
> > the slave-specific header operations on itself, causing
> > netdev_priv(bond_dev) (a struct bonding) to be incorrectly interpreted
> > as the slave's private-data type.
> > 
> > Patch 2 uses READ_ONCE when loading header_ops callbacks
> > to avoid races with concurrent updates.
> > 
>  With the READ_ONCE changes in a separate patch, does that mean
> that patch 1 by itself is subject to race conditions that would result
> in errors? If so, that's not acceptable, every patch must stand alone
> and not break the kernel.
> 
>  -J
> 
> > 
> > Fixes: 1284cd3a2b74 ("bonding: two small fixes for IPoIB support")
> > 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>
> > 
> > Kota Toda (2):
> >  net: bonding: fix type-confusion in bonding header_ops
> >  net: add READ_ONCE for header_ops callbacks
> > 
> >  drivers/net/bonding/bond_main.c | 67 ++++++++++++++++++++++++++++++++-
> >  include/linux/netdevice.h | 41 ++++++++++++++------
> >  include/net/bonding.h | 5 +++
> >  include/net/cfg802154.h | 2 +-
> >  net/core/neighbour.c | 6 +--
> >  net/ipv4/arp.c | 2 +-
> >  net/ipv6/ndisc.c | 2 +-
> >  7 files changed, 106 insertions(+), 19 deletions(-)
> > 
> > -- 
> > 2.53.0
> > 
> ---
>  -Jay Vosburgh, jv@jvosburgh.net
>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v4 0/2] net: bonding: fix type-confusion in bonding header_ops
  2026-03-05 20:59 ` [PATCH v4 0/2] net: bonding: fix type-confusion in bonding header_ops Jay Vosburgh
  2026-03-06  8:06   ` Jiayuan Chen
@ 2026-03-06  8:13   ` Jiayuan Chen
  2026-03-10 10:45   ` 戸田晃太
  2 siblings, 0 replies; 7+ messages in thread
From: Jiayuan Chen @ 2026-03-06  8:13 UTC (permalink / raw)
  To: Jay Vosburgh; +Cc: Jeff Garzik, netdev, linux-kernel, Yuki Koike

March 6, 2026 at 04:59, "Jay Vosburgh" <jv@jvosburgh.net mailto:jv@jvosburgh.net?to=%22Jay%20Vosburgh%22%20%3Cjv%40jvosburgh.net%3E > wrote:


> 
> Kota Toda <kota.toda@gmo-cybersecurity.com> wrote:
> 
> > 
> > In bond_setup_by_slave(), the slave’s header_ops are unconditionally
> > copied into the bonding device. As a result, the bonding device may invoke
> > the slave-specific header operations on itself, causing
> > netdev_priv(bond_dev) (a struct bonding) to be incorrectly interpreted
> > as the slave's private-data type.
> > 
> > This type-confusion bug can lead to out-of-bounds writes into the skb,
> > resulting in memory corruption.
> > 
>  A few days ago, Jiayuan Chen <jiayuan.chen@linux.dev> posted a
> fix for what sounds like the same problem[0]. Their solution appears to
> be much less complicated.
> 
>  I also wonder how this bug was discovered. The code in question
> hasn't changed in many years, and now there are two independent fixes
> within a week.

This issue has existed for years, but was likely masked by other bugs in the IP GRE
module.
https://syzkaller.appspot.com/bug?extid=4c63f36709a642f801c5
https://syzkaller.appspot.com/bug?id=77135d6c2fc52eff1b3c561912fbec39761e0461

The recent commit e67c577d8989 ("ipv4: ip_gre: make ipgre_header() robust"), which
introduced pskb_expand_head, has made the wrong type-casting issue more apparent.

v2 was sent:
https://lore.kernel.org/netdev/20260306021508.222062-1-jiayuan.chen@linux.dev/


> [0] https://lore.kernel.org/netdev/20260228095854.391093-1-jiayuan.chen@linux.dev/
> 
> > 
> > Patch 1 stores the slave's header_ops in struct bonding and sets
> > wrapper callbacks in bond_In bond_setup_by_slave(), the slave’s
> > header_ops are unconditionally
> > copied into the bonding device. As a result, the bonding device may invoke
> > the slave-specific header operations on itself, causing
> > netdev_priv(bond_dev) (a struct bonding) to be incorrectly interpreted
> > as the slave's private-data type.
> > 
> > Patch 2 uses READ_ONCE when loading header_ops callbacks
> > to avoid races with concurrent updates.
> > 
>  With the READ_ONCE changes in a separate patch, does that mean
> that patch 1 by itself is subject to race conditions that would result
> in errors? If so, that's not acceptable, every patch must stand alone
> and not break the kernel.
> 
>  -J
> 
> > 
> > Fixes: 1284cd3a2b74 ("bonding: two small fixes for IPoIB support")
> > 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>
> > 
> > Kota Toda (2):
> >  net: bonding: fix type-confusion in bonding header_ops
> >  net: add READ_ONCE for header_ops callbacks
> > 
> >  drivers/net/bonding/bond_main.c | 67 ++++++++++++++++++++++++++++++++-
> >  include/linux/netdevice.h | 41 ++++++++++++++------
> >  include/net/bonding.h | 5 +++
> >  include/net/cfg802154.h | 2 +-
> >  net/core/neighbour.c | 6 +--
> >  net/ipv4/arp.c | 2 +-
> >  net/ipv6/ndisc.c | 2 +-
> >  7 files changed, 106 insertions(+), 19 deletions(-)
> > 
> > -- 
> > 2.53.0
> > 
> ---
>  -Jay Vosburgh, jv@jvosburgh.net
>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v4 0/2] net: bonding: fix type-confusion in bonding header_ops
  2026-03-05 20:59 ` [PATCH v4 0/2] net: bonding: fix type-confusion in bonding header_ops Jay Vosburgh
  2026-03-06  8:06   ` Jiayuan Chen
  2026-03-06  8:13   ` Jiayuan Chen
@ 2026-03-10 10:45   ` 戸田晃太
  2 siblings, 0 replies; 7+ messages in thread
From: 戸田晃太 @ 2026-03-10 10:45 UTC (permalink / raw)
  To: Jay Vosburgh; +Cc: Jeff Garzik, netdev, linux-kernel, Yuki Koike, Jiayuan Chen

Thank you for your quick response.

I was not aware that such a patch had already been posted and got
approved. That patch looks like a good approach to me.
Given that, I am happy to withdraw my patch.

>  I also wonder how this bug was discovered.  The code in question
> hasn't changed in many years, and now there are two independent fixes
> within a week.

I found this bug through kernel fuzzing.

2026年3月6日(金) 5:59 Jay Vosburgh <jv@jvosburgh.net>:
>
> Kota Toda <kota.toda@gmo-cybersecurity.com> wrote:
>
> >In bond_setup_by_slave(), the slave’s header_ops are unconditionally
> >copied into the bonding device. As a result, the bonding device may invoke
> >the slave-specific header operations on itself, causing
> >netdev_priv(bond_dev) (a struct bonding) to be incorrectly interpreted
> >as the slave's private-data type.
> >
> >This type-confusion bug can lead to out-of-bounds writes into the skb,
> >resulting in memory corruption.
>
>         A few days ago, Jiayuan Chen <jiayuan.chen@linux.dev> posted a
> fix for what sounds like the same problem[0].  Their solution appears to
> be much less complicated.
>
>         I also wonder how this bug was discovered.  The code in question
> hasn't changed in many years, and now there are two independent fixes
> within a week.
>
> [0] https://lore.kernel.org/netdev/20260228095854.391093-1-jiayuan.chen@linux.dev/
>
> >Patch 1 stores the slave's header_ops in struct bonding and sets
> >wrapper callbacks in bond_In bond_setup_by_slave(), the slave’s
> >header_ops are unconditionally
> >copied into the bonding device. As a result, the bonding device may invoke
> >the slave-specific header operations on itself, causing
> >netdev_priv(bond_dev) (a struct bonding) to be incorrectly interpreted
> >as the slave's private-data type.
> >
> >Patch 2 uses READ_ONCE when loading header_ops callbacks
> >to avoid races with concurrent updates.
>
>         With the READ_ONCE changes in a separate patch, does that mean
> that patch 1 by itself is subject to race conditions that would result
> in errors?  If so, that's not acceptable, every patch must stand alone
> and not break the kernel.
>
>         -J
>
> >Fixes: 1284cd3a2b74 ("bonding: two small fixes for IPoIB support")
> >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>
> >
> >Kota Toda (2):
> >  net: bonding: fix type-confusion in bonding header_ops
> >  net: add READ_ONCE for header_ops callbacks
> >
> > drivers/net/bonding/bond_main.c | 67 ++++++++++++++++++++++++++++++++-
> > include/linux/netdevice.h       | 41 ++++++++++++++------
> > include/net/bonding.h           |  5 +++
> > include/net/cfg802154.h         |  2 +-
> > net/core/neighbour.c            |  6 +--
> > net/ipv4/arp.c                  |  2 +-
> > net/ipv6/ndisc.c                |  2 +-
> > 7 files changed, 106 insertions(+), 19 deletions(-)
> >
> >--
> >2.53.0
> >
> >
>
> ---
>         -Jay Vosburgh, jv@jvosburgh.net
>

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2026-03-10 10:45 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-05 11:07 [PATCH v4 0/2] net: bonding: fix type-confusion in bonding header_ops Kota Toda
2026-03-05 11:07 ` [PATCH v4 1/2] " Kota Toda
2026-03-05 11:07 ` [PATCH v4 2/2] net: read header_ops callbacks with READ_ONCE() Kota Toda
2026-03-05 20:59 ` [PATCH v4 0/2] net: bonding: fix type-confusion in bonding header_ops Jay Vosburgh
2026-03-06  8:06   ` Jiayuan Chen
2026-03-06  8:13   ` Jiayuan Chen
2026-03-10 10:45   ` 戸田晃太

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox