* [PATCH net v1] team: fix header_ops type confusion with non-Ethernet ports
@ 2026-03-14 6:23 Jiayuan Chen
2026-03-14 10:19 ` Eric Dumazet
0 siblings, 1 reply; 9+ messages in thread
From: Jiayuan Chen @ 2026-03-14 6:23 UTC (permalink / raw)
To: netdev
Cc: Jiayuan Chen, syzbot+3d8bc31c45e11450f24c, Jiayuan Chen,
Jiri Pirko, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, linux-kernel
From: Jiayuan Chen <jiayuan.chen@shopee.com>
Similar to commit 950803f72547 ("bonding: fix type confusion in bond_setup_by_slave()"),
team has the same class of header_ops type confusion.
For non-Ethernet ports, team_setup_by_port() copies port_dev->header_ops
directly. When the team device later calls dev_hard_header()/dev_parse_header(),
these callbacks can run with the team net_device instead of the real lower
device, so netdev_priv(dev) is interpreted as the wrong private type and
can crash.
Fix this by introducing team header_ops wrappers for create/parse, selecting
a team port under RCU, and calling the lower device callbacks with port->dev,
so each callback always sees the correct net_device context.
Reproducer:
set -euxo pipefail
for d in t0 b0 g0 d0; do
ip link del "$d" 2>/dev/null || true
done
modprobe bonding || true
modprobe team || true
modprobe ip_gre || true
modprobe dummy || true
ip link add d0 type dummy
ip addr add 10.10.10.1/24 dev d0
ip link set d0 up
ip link add g0 type gre local 10.10.10.1
ip link add b0 type bond mode active-backup
ip link add t0 type team
ip link set g0 master b0
ip link set b0 master t0
ip link set g0 up
ip link set b0 up
ip link set t0 up
Fixes: 8ff5105a2b9d ("team: add support for queue override by setting queue_id for port")
Reported-by: syzbot+3d8bc31c45e11450f24c@syzkaller.appspotmail.com
Closes: https://lore.kernel.org/all/69b46af7.050a0220.36eb34.000e.GAE@google.com/T/
Cc: Jiayuan Chen <jiayuan.chen@linux.dev>
Signed-off-by: Jiayuan Chen <jiayuan.chen@shopee.com>
---
drivers/net/team/team_core.c | 61 +++++++++++++++++++++++++++++++++++-
1 file changed, 60 insertions(+), 1 deletion(-)
diff --git a/drivers/net/team/team_core.c b/drivers/net/team/team_core.c
index b7282f5c9632..737bff5bf5ae 100644
--- a/drivers/net/team/team_core.c
+++ b/drivers/net/team/team_core.c
@@ -2058,6 +2058,65 @@ static const struct ethtool_ops team_ethtool_ops = {
* rt netlink interface
***********************/
+/* Caller must hold rcu_read_lock(). */
+static struct team_port *team_header_port_get_rcu(struct team *team, bool txable)
+{
+ struct team_port *port;
+
+ list_for_each_entry_rcu(port, &team->port_list, list) {
+ if (!txable || team_port_txable(port))
+ return port;
+ }
+
+ return NULL;
+}
+
+static int team_header_create(struct sk_buff *skb, struct net_device *team_dev,
+ unsigned short type, const void *daddr,
+ const void *saddr, unsigned int len)
+{
+ struct team *team = netdev_priv(team_dev);
+ const struct header_ops *port_ops;
+ struct team_port *port;
+ int ret = 0;
+
+ rcu_read_lock();
+ port = team_header_port_get_rcu(team, true);
+ if (port) {
+ port_ops = READ_ONCE(port->dev->header_ops);
+ if (port_ops && port_ops->create)
+ ret = port_ops->create(skb, port->dev,
+ type, daddr, saddr, len);
+ }
+ rcu_read_unlock();
+ return ret;
+}
+
+static int team_header_parse(const struct sk_buff *skb, unsigned char *haddr)
+{
+ struct team *team = netdev_priv(skb->dev);
+ const struct header_ops *port_ops;
+ struct team_port *port;
+ int ret = 0;
+
+ rcu_read_lock();
+ port = team_header_port_get_rcu(team, true);
+ if (!port)
+ port = team_header_port_get_rcu(team, false);
+ if (port) {
+ port_ops = READ_ONCE(port->dev->header_ops);
+ if (port_ops && port_ops->parse)
+ ret = port_ops->parse(skb, haddr);
+ }
+ rcu_read_unlock();
+ return ret;
+}
+
+static const struct header_ops team_header_ops = {
+ .create = team_header_create,
+ .parse = team_header_parse,
+};
+
static void team_setup_by_port(struct net_device *dev,
struct net_device *port_dev)
{
@@ -2066,7 +2125,7 @@ static void team_setup_by_port(struct net_device *dev,
if (port_dev->type == ARPHRD_ETHER)
dev->header_ops = team->header_ops_cache;
else
- dev->header_ops = port_dev->header_ops;
+ dev->header_ops = port_dev->header_ops ? &team_header_ops : NULL;
dev->type = port_dev->type;
dev->hard_header_len = port_dev->hard_header_len;
dev->needed_headroom = port_dev->needed_headroom;
--
2.43.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH net v1] team: fix header_ops type confusion with non-Ethernet ports
2026-03-14 6:23 [PATCH net v1] team: fix header_ops type confusion with non-Ethernet ports Jiayuan Chen
@ 2026-03-14 10:19 ` Eric Dumazet
2026-03-14 11:14 ` Eric Dumazet
0 siblings, 1 reply; 9+ messages in thread
From: Eric Dumazet @ 2026-03-14 10:19 UTC (permalink / raw)
To: Jiayuan Chen
Cc: netdev, Jiayuan Chen, syzbot+3d8bc31c45e11450f24c, Jiri Pirko,
Andrew Lunn, David S. Miller, Jakub Kicinski, Paolo Abeni,
linux-kernel
On Sat, Mar 14, 2026 at 7:23 AM Jiayuan Chen <jiayuan.chen@linux.dev> wrote:
>
> From: Jiayuan Chen <jiayuan.chen@shopee.com>
>
> Similar to commit 950803f72547 ("bonding: fix type confusion in bond_setup_by_slave()"),
> team has the same class of header_ops type confusion.
>
> For non-Ethernet ports, team_setup_by_port() copies port_dev->header_ops
> directly. When the team device later calls dev_hard_header()/dev_parse_header(),
> these callbacks can run with the team net_device instead of the real lower
> device, so netdev_priv(dev) is interpreted as the wrong private type and
> can crash.
>
> Fix this by introducing team header_ops wrappers for create/parse, selecting
> a team port under RCU, and calling the lower device callbacks with port->dev,
> so each callback always sees the correct net_device context.
>
> Reproducer:
> set -euxo pipefail
> for d in t0 b0 g0 d0; do
> ip link del "$d" 2>/dev/null || true
> done
>
> modprobe bonding || true
> modprobe team || true
> modprobe ip_gre || true
> modprobe dummy || true
> ip link add d0 type dummy
> ip addr add 10.10.10.1/24 dev d0
> ip link set d0 up
> ip link add g0 type gre local 10.10.10.1
> ip link add b0 type bond mode active-backup
> ip link add t0 type team
> ip link set g0 master b0
> ip link set b0 master t0
> ip link set g0 up
> ip link set b0 up
> ip link set t0 up
I have a bad feeling about the bonding patch.
Let's not copy paste it to team before clearing these feelings.
Can you add tests with a stack of two bonding devices ?
Because I think bond_header_parse() would enter an infinite recursion loop,
because skb->dev always points to the head of the tree ?
Unless I am missing something obvious.
Sorry this is weekend time.
static int bond_header_parse(const struct sk_buff *skb, unsigned char *haddr)
{
struct bonding *bond = netdev_priv(skb->dev);
const struct header_ops *slave_ops;
struct slave *slave;
int ret = 0;
rcu_read_lock();
slave = rcu_dereference(bond->curr_active_slave);
if (slave) {
slave_ops = READ_ONCE(slave->dev->header_ops);
if (slave_ops && slave_ops->parse)
ret = slave_ops->parse(skb, haddr);
}
rcu_read_unlock();
return ret;
}
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH net v1] team: fix header_ops type confusion with non-Ethernet ports
2026-03-14 10:19 ` Eric Dumazet
@ 2026-03-14 11:14 ` Eric Dumazet
2026-03-14 11:41 ` Jiayuan Chen
0 siblings, 1 reply; 9+ messages in thread
From: Eric Dumazet @ 2026-03-14 11:14 UTC (permalink / raw)
To: Jiayuan Chen
Cc: netdev, Jiayuan Chen, syzbot+3d8bc31c45e11450f24c, Jiri Pirko,
Andrew Lunn, David S. Miller, Jakub Kicinski, Paolo Abeni,
linux-kernel
On Sat, Mar 14, 2026 at 11:19 AM Eric Dumazet <edumazet@google.com> wrote:
>
> On Sat, Mar 14, 2026 at 7:23 AM Jiayuan Chen <jiayuan.chen@linux.dev> wrote:
> >
> > From: Jiayuan Chen <jiayuan.chen@shopee.com>
> >
> > Similar to commit 950803f72547 ("bonding: fix type confusion in bond_setup_by_slave()"),
> > team has the same class of header_ops type confusion.
> >
> > For non-Ethernet ports, team_setup_by_port() copies port_dev->header_ops
> > directly. When the team device later calls dev_hard_header()/dev_parse_header(),
> > these callbacks can run with the team net_device instead of the real lower
> > device, so netdev_priv(dev) is interpreted as the wrong private type and
> > can crash.
> >
> > Fix this by introducing team header_ops wrappers for create/parse, selecting
> > a team port under RCU, and calling the lower device callbacks with port->dev,
> > so each callback always sees the correct net_device context.
> >
> > Reproducer:
> > set -euxo pipefail
> > for d in t0 b0 g0 d0; do
> > ip link del "$d" 2>/dev/null || true
> > done
> >
> > modprobe bonding || true
> > modprobe team || true
> > modprobe ip_gre || true
> > modprobe dummy || true
> > ip link add d0 type dummy
> > ip addr add 10.10.10.1/24 dev d0
> > ip link set d0 up
> > ip link add g0 type gre local 10.10.10.1
> > ip link add b0 type bond mode active-backup
> > ip link add t0 type team
> > ip link set g0 master b0
> > ip link set b0 master t0
> > ip link set g0 up
> > ip link set b0 up
> > ip link set t0 up
>
> I have a bad feeling about the bonding patch.
>
> Let's not copy paste it to team before clearing these feelings.
>
> Can you add tests with a stack of two bonding devices ?
>
> Because I think bond_header_parse() would enter an infinite recursion loop,
> because skb->dev always points to the head of the tree ?
> Unless I am missing something obvious.
> Sorry this is weekend time.
>
I think the following (WIP, untested) patch is needed.
I will complete it and send it ASAP.
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 707419270ebf217a71b0593880c7a9a1481b7171..33f414d03ab913c58cf2406a4ab25e611c528159
100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -1530,9 +1530,11 @@ static int bond_header_create(struct sk_buff
*skb, struct net_device *bond_dev,
return ret;
}
-static int bond_header_parse(const struct sk_buff *skb, unsigned char *haddr)
+static int bond_header_parse(const struct sk_buff *skb,
+ const struct net_device *dev,
+ unsigned char *haddr)
{
- struct bonding *bond = netdev_priv(skb->dev);
+ struct bonding *bond = netdev_priv(dev);
const struct header_ops *slave_ops;
struct slave *slave;
int ret = 0;
@@ -1542,7 +1544,7 @@ static int bond_header_parse(const struct
sk_buff *skb, unsigned char *haddr)
if (slave) {
slave_ops = READ_ONCE(slave->dev->header_ops);
if (slave_ops && slave_ops->parse)
- ret = slave_ops->parse(skb, haddr);
+ ret = slave_ops->parse(skb, slave->dev, haddr);
}
rcu_read_unlock();
return ret;
diff --git a/include/linux/etherdevice.h b/include/linux/etherdevice.h
index 9a1eacf35d37087ba8877bf31c017445929041ed..df8f88f63a7063fbd1df5248d2fc02c859a7bc74
100644
--- a/include/linux/etherdevice.h
+++ b/include/linux/etherdevice.h
@@ -42,7 +42,8 @@ extern const struct header_ops eth_header_ops;
int eth_header(struct sk_buff *skb, struct net_device *dev, unsigned
short type,
const void *daddr, const void *saddr, unsigned len);
-int eth_header_parse(const struct sk_buff *skb, unsigned char *haddr);
+int eth_header_parse(const struct sk_buff *skb, const struct net_device *dev,
+ unsigned char *haddr);
int eth_header_cache(const struct neighbour *neigh, struct hh_cache *hh,
__be16 type);
void eth_header_cache_update(struct hh_cache *hh, const struct net_device *dev,
diff --git a/include/linux/if_ether.h b/include/linux/if_ether.h
index 61b7335aa037c7232a0caa45572043057c02dde3..ca9afa824aa4faf832658043bda6fb430633e476
100644
--- a/include/linux/if_ether.h
+++ b/include/linux/if_ether.h
@@ -40,7 +40,8 @@ static inline struct ethhdr *inner_eth_hdr(const
struct sk_buff *skb)
return (struct ethhdr *)skb_inner_mac_header(skb);
}
-int eth_header_parse(const struct sk_buff *skb, unsigned char *haddr);
+int eth_header_parse(const struct sk_buff *skb, const struct net_device *dev,
+ unsigned char *haddr);
extern ssize_t sysfs_format_mac(char *buf, const unsigned char *addr, int len);
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index d7aac6f185bcab8a93a204c349272fc7c1b15ee7..7ca01eb3f7d2b22a188502583dc95121adff7cc9
100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -311,7 +311,9 @@ struct header_ops {
int (*create) (struct sk_buff *skb, struct net_device *dev,
unsigned short type, const void *daddr,
const void *saddr, unsigned int len);
- int (*parse)(const struct sk_buff *skb, unsigned char *haddr);
+ int (*parse)(const struct sk_buff *skb,
+ const struct net_device *dev,
+ unsigned char *haddr);
int (*cache)(const struct neighbour *neigh, struct
hh_cache *hh, __be16 type);
void (*cache_update)(struct hh_cache *hh,
const struct net_device *dev,
@@ -3445,7 +3447,7 @@ static inline int dev_parse_header(const struct
sk_buff *skb,
if (!dev->header_ops || !dev->header_ops->parse)
return 0;
- return dev->header_ops->parse(skb, haddr);
+ return dev->header_ops->parse(skb, dev, haddr);
}
static inline __be16 dev_parse_header_protocol(const struct sk_buff *skb)
diff --git a/net/ethernet/eth.c b/net/ethernet/eth.c
index 13a63b48b7eeb896dfe98eb0070a261eed2c384b..9d159d1cc57d42747f794cdf43fe0ccaf04818b2
100644
--- a/net/ethernet/eth.c
+++ b/net/ethernet/eth.c
@@ -198,7 +198,8 @@ EXPORT_SYMBOL(eth_type_trans);
* @skb: packet to extract header from
* @haddr: destination buffer
*/
-int eth_header_parse(const struct sk_buff *skb, unsigned char *haddr)
+int eth_header_parse(const struct sk_buff *skb, const struct net_device *dev,
+ unsigned char *haddr)
{
const struct ethhdr *eth = eth_hdr(skb);
memcpy(haddr, eth->h_source, ETH_ALEN);
diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c
index e13244729ad8d5b1c2b9c483d25bff0e438134b5..35f0baa99d4092fd499a4795f7f52db33a1fe4e2
100644
--- a/net/ipv4/ip_gre.c
+++ b/net/ipv4/ip_gre.c
@@ -919,7 +919,8 @@ static int ipgre_header(struct sk_buff *skb,
struct net_device *dev,
return -(t->hlen + sizeof(*iph));
}
-static int ipgre_header_parse(const struct sk_buff *skb, unsigned char *haddr)
+static int ipgre_header_parse(const struct sk_buff *skb, const struct
net_device *dev,
+ unsigned char *haddr)
{
const struct iphdr *iph = (const struct iphdr *) skb_mac_header(skb);
memcpy(haddr, &iph->saddr, 4);
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH net v1] team: fix header_ops type confusion with non-Ethernet ports
2026-03-14 11:14 ` Eric Dumazet
@ 2026-03-14 11:41 ` Jiayuan Chen
2026-03-14 11:46 ` Eric Dumazet
0 siblings, 1 reply; 9+ messages in thread
From: Jiayuan Chen @ 2026-03-14 11:41 UTC (permalink / raw)
To: Eric Dumazet
Cc: netdev, Jiayuan Chen, syzbot+3d8bc31c45e11450f24c, Jiri Pirko,
Andrew Lunn, David S. Miller, Jakub Kicinski, Paolo Abeni,
linux-kernel
On 3/14/26 7:14 PM, Eric Dumazet wrote:
> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
> index 707419270ebf217a71b0593880c7a9a1481b7171..33f414d03ab913c58cf2406a4ab25e611c528159
> 100644
> --- a/drivers/net/bonding/bond_main.c
> +++ b/drivers/net/bonding/bond_main.c
> @@ -1530,9 +1530,11 @@ static int bond_header_create(struct sk_buff
> *skb, struct net_device *bond_dev,
> return ret;
> }
>
> -static int bond_header_parse(const struct sk_buff *skb, unsigned char *haddr)
> +static int bond_header_parse(const struct sk_buff *skb,
> + const struct net_device *dev,
> + unsigned char *haddr)
> {
> - struct bonding *bond = netdev_priv(skb->dev);
> + struct bonding *bond = netdev_priv(dev);
> const struct header_ops *slave_ops;
> struct slave *slave;
> int ret = 0;
> @@ -1542,7 +1544,7 @@ static int bond_header_parse(const struct
> sk_buff *skb, unsigned char *haddr)
> if (slave) {
> slave_ops = READ_ONCE(slave->dev->header_ops);
> if (slave_ops && slave_ops->parse)
> - ret = slave_ops->parse(skb, haddr);
> + ret = slave_ops->parse(skb, slave->dev, haddr);
> }
> rcu_read_unlock();
> return ret;
> diff --git a/include/linux/etherdevice.h b/include/linux/etherdevice.h
> index 9a1eacf35d37087ba8877bf31c017445929041ed..df8f88f63a7063fbd1df5248d2fc02c859a7bc74
> 100644
> --- a/include/linux/etherdevice.h
> +++ b/include/linux/etherdevice.h
> @@ -42,7 +42,8 @@ extern const struct header_ops eth_header_ops;
>
> int eth_header(struct sk_buff *skb, struct net_device *dev, unsigned
> short type,
> const void *daddr, const void *saddr, unsigned len);
> -int eth_header_parse(const struct sk_buff *skb, unsigned char *haddr);
> +int eth_header_parse(const struct sk_buff *skb, const struct net_device *dev,
> + unsigned char *haddr);
> int eth_header_cache(const struct neighbour *neigh, struct hh_cache *hh,
> __be16 type);
> void eth_header_cache_update(struct hh_cache *hh, const struct net_device *dev,
> diff --git a/include/linux/if_ether.h b/include/linux/if_ether.h
> index 61b7335aa037c7232a0caa45572043057c02dde3..ca9afa824aa4faf832658043bda6fb430633e476
> 100644
> --- a/include/linux/if_ether.h
> +++ b/include/linux/if_ether.h
> @@ -40,7 +40,8 @@ static inline struct ethhdr *inner_eth_hdr(const
> struct sk_buff *skb)
> return (struct ethhdr *)skb_inner_mac_header(skb);
> }
>
> -int eth_header_parse(const struct sk_buff *skb, unsigned char *haddr);
> +int eth_header_parse(const struct sk_buff *skb, const struct net_device *dev,
> + unsigned char *haddr);
>
> extern ssize_t sysfs_format_mac(char *buf, const unsigned char *addr, int len);
>
> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> index d7aac6f185bcab8a93a204c349272fc7c1b15ee7..7ca01eb3f7d2b22a188502583dc95121adff7cc9
> 100644
> --- a/include/linux/netdevice.h
> +++ b/include/linux/netdevice.h
> @@ -311,7 +311,9 @@ struct header_ops {
> int (*create) (struct sk_buff *skb, struct net_device *dev,
> unsigned short type, const void *daddr,
> const void *saddr, unsigned int len);
> - int (*parse)(const struct sk_buff *skb, unsigned char *haddr);
> + int (*parse)(const struct sk_buff *skb,
> + const struct net_device *dev,
> + unsigned char *haddr);
> int (*cache)(const struct neighbour *neigh, struct
> hh_cache *hh, __be16 type);
> void (*cache_update)(struct hh_cache *hh,
> const struct net_device *dev,
> @@ -3445,7 +3447,7 @@ static inline int dev_parse_header(const struct
> sk_buff *skb,
>
> if (!dev->header_ops || !dev->header_ops->parse)
> return 0;
> - return dev->header_ops->parse(skb, haddr);
> + return dev->header_ops->parse(skb, dev, haddr);
> }
>
> static inline __be16 dev_parse_header_protocol(const struct sk_buff *skb)
> diff --git a/net/ethernet/eth.c b/net/ethernet/eth.c
> index 13a63b48b7eeb896dfe98eb0070a261eed2c384b..9d159d1cc57d42747f794cdf43fe0ccaf04818b2
> 100644
> --- a/net/ethernet/eth.c
> +++ b/net/ethernet/eth.c
> @@ -198,7 +198,8 @@ EXPORT_SYMBOL(eth_type_trans);
> * @skb: packet to extract header from
> * @haddr: destination buffer
> */
> -int eth_header_parse(const struct sk_buff *skb, unsigned char *haddr)
> +int eth_header_parse(const struct sk_buff *skb, const struct net_device *dev,
> + unsigned char *haddr)
> {
> const struct ethhdr *eth = eth_hdr(skb);
> memcpy(haddr, eth->h_source, ETH_ALEN);
> diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c
> index e13244729ad8d5b1c2b9c483d25bff0e438134b5..35f0baa99d4092fd499a4795f7f52db33a1fe4e2
> 100644
> --- a/net/ipv4/ip_gre.c
> +++ b/net/ipv4/ip_gre.c
> @@ -919,7 +919,8 @@ static int ipgre_header(struct sk_buff *skb,
> struct net_device *dev,
> return -(t->hlen + sizeof(*iph));
> }
>
> -static int ipgre_header_parse(const struct sk_buff *skb, unsigned char *haddr)
> +static int ipgre_header_parse(const struct sk_buff *skb, const struct
> net_device *dev,
> + unsigned char *haddr)
> {
> const struct iphdr *iph = (const struct iphdr *) skb_mac_header(skb);
> memcpy(haddr, &iph->saddr, 4);
Thanks,
I reproduced the infinite recursion you described with stacked bonds
(bond1 -> bond0 -> gre0). With an AF_PACKET SOCK_DGRAM socket on bond1
and an inbound GRE packet, bond_header_parse() recurses endlessly
because skb->dev always points to bond1.
I can verify this patch, but changing the parse() signature touches
quite a few places beyond what's in the diff - net/mac802154/iface.c,
net/phonet/af_phonet.c, drivers/firewire/net.c all have their own
parse() implementations that need updating, plus eth_header_parse() is
declared in both etherdevice.h and if_ether.h.
I'm wondering if adding a separate callback (e.g. dev_parse) to
struct header_ops might be a cleaner approach - dev_parse_header()
would check for the new callback first and fall back to the existing
parse(). That way only bond (and team) need to implement it, and all
other drivers remain untouched. The tradeoff is one extra NULL check
in the receive path, which should be negligible.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH net v1] team: fix header_ops type confusion with non-Ethernet ports
2026-03-14 11:41 ` Jiayuan Chen
@ 2026-03-14 11:46 ` Eric Dumazet
2026-03-14 11:51 ` Jiayuan Chen
0 siblings, 1 reply; 9+ messages in thread
From: Eric Dumazet @ 2026-03-14 11:46 UTC (permalink / raw)
To: Jiayuan Chen
Cc: netdev, Jiayuan Chen, syzbot+3d8bc31c45e11450f24c, Jiri Pirko,
Andrew Lunn, David S. Miller, Jakub Kicinski, Paolo Abeni,
linux-kernel
On Sat, Mar 14, 2026 at 12:41 PM Jiayuan Chen <jiayuan.chen@linux.dev> wrote:
>
>
>
> Thanks,
>
> I reproduced the infinite recursion you described with stacked bonds
> (bond1 -> bond0 -> gre0). With an AF_PACKET SOCK_DGRAM socket on bond1
> and an inbound GRE packet, bond_header_parse() recurses endlessly
> because skb->dev always points to bond1.
>
> I can verify this patch, but changing the parse() signature touches
> quite a few places beyond what's in the diff - net/mac802154/iface.c,
> net/phonet/af_phonet.c, drivers/firewire/net.c all have their own
> parse() implementations that need updating, plus eth_header_parse() is
> declared in both etherdevice.h and if_ether.h.
>
> I'm wondering if adding a separate callback (e.g. dev_parse) to
> struct header_ops might be a cleaner approach - dev_parse_header()
> would check for the new callback first and fall back to the existing
> parse(). That way only bond (and team) need to implement it, and all
> other drivers remain untouched. The tradeoff is one extra NULL check
> in the receive path, which should be negligible.
My patch is ready and already running our tests before I submit it.
git show | diffstat -p1
drivers/firewire/net.c | 5 +++--
drivers/net/bonding/bond_main.c | 8 +++++---
include/linux/etherdevice.h | 3 ++-
include/linux/if_ether.h | 3 ++-
include/linux/netdevice.h | 6 ++++--
net/ethernet/eth.c | 3 ++-
net/ipv4/ip_gre.c | 3 ++-
net/mac802154/iface.c | 4 +++-
net/phonet/af_phonet.c | 5 ++++-
9 files changed, 27 insertions(+), 13 deletions(-)
I see no issue with this fix, ->parse() is hardly in a fast path anyway.
Note that ->create() already has a 'struct net_device' pointer.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH net v1] team: fix header_ops type confusion with non-Ethernet ports
2026-03-14 11:46 ` Eric Dumazet
@ 2026-03-14 11:51 ` Jiayuan Chen
2026-03-14 11:53 ` Eric Dumazet
0 siblings, 1 reply; 9+ messages in thread
From: Jiayuan Chen @ 2026-03-14 11:51 UTC (permalink / raw)
To: Eric Dumazet
Cc: netdev, Jiayuan Chen, syzbot+3d8bc31c45e11450f24c, Jiri Pirko,
Andrew Lunn, David S. Miller, Jakub Kicinski, Paolo Abeni,
linux-kernel
On 3/14/26 7:46 PM, Eric Dumazet wrote:
>> Thanks,
>>
>> I reproduced the infinite recursion you described with stacked bonds
>> (bond1 -> bond0 -> gre0). With an AF_PACKET SOCK_DGRAM socket on bond1
>> and an inbound GRE packet, bond_header_parse() recurses endlessly
>> because skb->dev always points to bond1.
>>
>> I can verify this patch, but changing the parse() signature touches
>> quite a few places beyond what's in the diff - net/mac802154/iface.c,
>> net/phonet/af_phonet.c, drivers/firewire/net.c all have their own
>> parse() implementations that need updating, plus eth_header_parse() is
>> declared in both etherdevice.h and if_ether.h.
>>
>> I'm wondering if adding a separate callback (e.g. dev_parse) to
>> struct header_ops might be a cleaner approach - dev_parse_header()
>> would check for the new callback first and fall back to the existing
>> parse(). That way only bond (and team) need to implement it, and all
>> other drivers remain untouched. The tradeoff is one extra NULL check
>> in the receive path, which should be negligible.
> My patch is ready and already running our tests before I submit it.
>
> git show | diffstat -p1
> drivers/firewire/net.c | 5 +++--
> drivers/net/bonding/bond_main.c | 8 +++++---
> include/linux/etherdevice.h | 3 ++-
> include/linux/if_ether.h | 3 ++-
> include/linux/netdevice.h | 6 ++++--
> net/ethernet/eth.c | 3 ++-
> net/ipv4/ip_gre.c | 3 ++-
> net/mac802154/iface.c | 4 +++-
> net/phonet/af_phonet.c | 5 ++++-
> 9 files changed, 27 insertions(+), 13 deletions(-)
>
> I see no issue with this fix, ->parse() is hardly in a fast path anyway.
>
> Note that ->create() already has a 'struct net_device' pointer.
Thanks for the quick patch.
I also verified it fixes the recursion issue after modifying all signatures.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH net v1] team: fix header_ops type confusion with non-Ethernet ports
2026-03-14 11:51 ` Jiayuan Chen
@ 2026-03-14 11:53 ` Eric Dumazet
2026-03-14 11:57 ` Eric Dumazet
2026-03-14 12:05 ` Jiayuan Chen
0 siblings, 2 replies; 9+ messages in thread
From: Eric Dumazet @ 2026-03-14 11:53 UTC (permalink / raw)
To: Jiayuan Chen
Cc: netdev, Jiayuan Chen, syzbot+3d8bc31c45e11450f24c, Jiri Pirko,
Andrew Lunn, David S. Miller, Jakub Kicinski, Paolo Abeni,
linux-kernel
On Sat, Mar 14, 2026 at 12:51 PM Jiayuan Chen <jiayuan.chen@linux.dev> wrote:
>
> Thanks for the quick patch.
>
> I also verified it fixes the recursion issue after modifying all signatures.
Could you turn your test in an official patch for tools/testing/selftests ?
Thanks !
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH net v1] team: fix header_ops type confusion with non-Ethernet ports
2026-03-14 11:53 ` Eric Dumazet
@ 2026-03-14 11:57 ` Eric Dumazet
2026-03-14 12:05 ` Jiayuan Chen
1 sibling, 0 replies; 9+ messages in thread
From: Eric Dumazet @ 2026-03-14 11:57 UTC (permalink / raw)
To: Jiayuan Chen
Cc: netdev, Jiayuan Chen, syzbot+3d8bc31c45e11450f24c, Jiri Pirko,
Andrew Lunn, David S. Miller, Jakub Kicinski, Paolo Abeni,
linux-kernel
On Sat, Mar 14, 2026 at 12:53 PM Eric Dumazet <edumazet@google.com> wrote:
>
> On Sat, Mar 14, 2026 at 12:51 PM Jiayuan Chen <jiayuan.chen@linux.dev> wrote:
> >
> > Thanks for the quick patch.
> >
> > I also verified it fixes the recursion issue after modifying all signatures.
>
> Could you turn your test in an official patch for tools/testing/selftests ?
>
> Thanks !
Fix sent for review :
https://lore.kernel.org/netdev/20260314115650.3646361-1-edumazet@google.com/T/#u
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH net v1] team: fix header_ops type confusion with non-Ethernet ports
2026-03-14 11:53 ` Eric Dumazet
2026-03-14 11:57 ` Eric Dumazet
@ 2026-03-14 12:05 ` Jiayuan Chen
1 sibling, 0 replies; 9+ messages in thread
From: Jiayuan Chen @ 2026-03-14 12:05 UTC (permalink / raw)
To: Eric Dumazet, Jiayuan Chen
Cc: netdev, Jiayuan Chen, syzbot+3d8bc31c45e11450f24c, Jiri Pirko,
Andrew Lunn, David S. Miller, Jakub Kicinski, Paolo Abeni,
linux-kernel
On 3/14/26 7:53 PM, Eric Dumazet wrote:
> On Sat, Mar 14, 2026 at 12:51 PM Jiayuan Chen <jiayuan.chen@linux.dev> wrote:
>> Thanks for the quick patch.
>>
>> I also verified it fixes the recursion issue after modifying all signatures.
> Could you turn your test in an official patch for tools/testing/selftests ?
>
> Thanks !
I will send a selftest for this to the net tree if this patch hasn't
been merged to net-next yet.
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2026-03-14 12:07 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-14 6:23 [PATCH net v1] team: fix header_ops type confusion with non-Ethernet ports Jiayuan Chen
2026-03-14 10:19 ` Eric Dumazet
2026-03-14 11:14 ` Eric Dumazet
2026-03-14 11:41 ` Jiayuan Chen
2026-03-14 11:46 ` Eric Dumazet
2026-03-14 11:51 ` Jiayuan Chen
2026-03-14 11:53 ` Eric Dumazet
2026-03-14 11:57 ` Eric Dumazet
2026-03-14 12:05 ` Jiayuan Chen
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox