* [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