Re: [PATCH net v1] team: fix header_ops type confusion with non-Ethernet ports

[Eric Dumazet]

On Sat, Mar 14, 2026 at 7:23 AM Jiayuan Chen <jiayuan.chen@xxxxxxxxx> wrote:
>
> From: Jiayuan Chen <jiayuan.chen@xxxxxxxxxx>
>
> 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;
}