Re: [PATCH net 1/2] bonding: reject frames with insufficient headroom in bond_header_create
From: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
Date: 2026-08-26 02:02:17
Also in:
stable
Qihang wrote:
From: Qihang Tang <redacted>
Thanks for working on this Qihang. Minor: a patch series should generally have a cover letter
AF_PACKET SOCK_DGRAM sends reserve skb headroom from a snapshot of bond_dev->hard_header_len. A concurrent bond type change can switch the active slave to one with a larger hard_header_len between that snapshot and bond_header_create(), so the slave's create() pushes or writes past skb->head.
Are we certain that bond and team are the only devices that implement header_ops->create?
The hard_header_len snapshot series that fixed the SOCK_RAW send paths
deferred this SOCK_DGRAM race: dev->header_ops is the stable
bond_header_ops, so snapshotting header_ops in the caller does not help.
Reject the frame if skb headroom is smaller than the active slave's
hard_header_len, before delegating under the existing rcu_read_lock.
Fixes: 950803f72547 ("bonding: fix type confusion in bond_setup_by_slave()")What's the rationale behind blaming this SHA1?
quoted hunk ↗ jump to hunk
Cc: stable@vger.kernel.org Cc: Willem de Bruijn <willemb@google.com> Signed-off-by: Qihang Tang <redacted> --- drivers/net/bonding/bond_main.c | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-)diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c index 522eab060f9e..9ec663610dfd 100644 --- a/drivers/net/bonding/bond_main.c +++ b/drivers/net/bonding/bond_main.c@@ -1524,10 +1524,24 @@ static int bond_header_create(struct sk_buff *skb, struct net_device *bond_dev, slave = rcu_dereference(bond->curr_active_slave); if (slave) { slave_ops = READ_ONCE(slave->dev->header_ops); - if (slave_ops && slave_ops->create) + if (slave_ops && slave_ops->create) { + unsigned int hlen = READ_ONCE(slave->dev->hard_header_len); + + /* Headroom was reserved from a snapshot of + * bond_dev->hard_header_len that may predate this + * slave (concurrent bond type change); reject if + * insufficient for the slave's create(), which + * pushes its own hlen. + */ + if (skb_headroom(skb) < hlen) { + ret = -EINVAL; + goto unlock; + } ret = slave_ops->create(skb, slave->dev, type, daddr, saddr, len); + }
Ideally we could fix this in dev_hard_header or PF_PACKET itself, rather than in each implementation that's in scope. But there seems no way around holding this rcu reference on the slave device while re-checking.
} +unlock: rcu_read_unlock(); return ret; } -- 2.50.1 (Apple Git-155)