Re: [PATCH net 1/2] bonding: reject frames with insufficient headroom in bond_header_create

[Willem de Bruijn <willemdebruijn.kernel@gmail.com>]

Qihang wrote:
> Yeah, agreed on steady state.
> 
> One thing to flag: packet_snd snapshots dev->hard_header_len at
> af_packet.c:3023 (no lock) but pushes the header at :3059 onto whatever
> curr_active_slave is then. A concurrent enslave that bumps it in between
> -- first non-Ethernet slave 14->24, or a bigger-header failover -- can

It does appear that all slaves need to be of the same type, in
bond_enslave:

        if (!bond_has_slaves(bond)) {
                ...
        } else if (bond_dev->type != slave_dev->type) {
                SLAVE_NL_ERR(bond_dev, slave_dev, extack,
                             "Device type is different from other slaves");
        }

> leave the in-flight frame short and trip skb_under_panic (oops/panic,
> not silent corruption; local DoS for a privileged user).

But the same type can still create different hard_header_len, e.g.,
with GRE devices.

I had expected that bond slave changes, such as bond_enslave, would
flush the stack using synchronize_net(). For instance when updating
the valid slaves for non-primary bonds, in bond_set_slave_arr.

But it does appear that a packet may see two different
bond->hard_header_len in a single packet_snd.

> Happy to drop v1, or hand over the create-time check if you want it closed.

Much appreciate your help on this, Qihang.