On Thu, Apr 27, 2017 at 07:29:34PM +0200, Paolo Abeni wrote:
> On slave list updates, the bonding driver computes its hard_header_len
> as the maximum of all enslaved devices's hard_header_len.
> If the slave list is empty, e.g. on last enslaved device removal,
> ETH_HLEN is used.
>
> Since the bonding header_ops are set only when the first enslaved
> device is attached, the above can lead to header_ops->create()
> being called with the wrong skb headroom in place.
>
> If bond0 is configured on top of ipoib devices, with the
> following commands:
>
> ifup bond0
> for slave in $BOND_SLAVES_LIST; do
> ip link set dev $slave nomaster
> done
> ping -c 1 <ip on bond0 subnet>
>
> we will obtain a skb_under_panic() with a similar call trace:
> skb_push+0x3d/0x40
> push_pseudo_header+0x17/0x30 [ib_ipoib]
> ipoib_hard_header+0x4e/0x80 [ib_ipoib]
> arp_create+0x12f/0x220
> arp_send_dst.part.19+0x28/0x50
> arp_solicit+0x115/0x290
> neigh_probe+0x4d/0x70
> __neigh_event_send+0xa7/0x230
> neigh_resolve_output+0x12e/0x1c0
> ip_finish_output2+0x14b/0x390
> ip_finish_output+0x136/0x1e0
> ip_output+0x76/0xe0
> ip_local_out+0x35/0x40
> ip_send_skb+0x19/0x40
> ip_push_pending_frames+0x33/0x40
> raw_sendmsg+0x7d3/0xb50
> inet_sendmsg+0x31/0xb0
> sock_sendmsg+0x38/0x50
> SYSC_sendto+0x102/0x190
> SyS_sendto+0xe/0x10
> do_syscall_64+0x67/0x180
> entry_SYSCALL64_slow_path+0x25/0x25
>
> This change addresses the issue avoiding updating the bonding device
> hard_header_len when the slaves list become empty, forbidding to
> shrink it below the value used by header_ops->create().
>
> The bug is there since commit 54ef31371407 ("[PATCH] bonding: Handle large
> hard_header_len") but the panic can be triggered only since
> commit fc791b633515 ("IB/ipoib: move back IB LL address into the hard
> header").
>
> Reported-by: Norbert P <n...@physik.uzh.ch>
> Fixes: 54ef31371407 ("[PATCH] bonding: Handle large hard_header_len")
> Fixes: fc791b633515 ("IB/ipoib: move back IB LL address into the hard header")
> Signed-off-by: Marcelo Ricardo Leitner <marcelo.leit...@gmail.com>
> Signed-off-by: Paolo Abeni <pab...@redhat.com>
> ---
Thanks Paolo.
> drivers/net/bonding/bond_main.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
> index 8a4ba8b..34481c9 100644
> --- a/drivers/net/bonding/bond_main.c
> +++ b/drivers/net/bonding/bond_main.c
> @@ -1104,11 +1104,11 @@ static void bond_compute_features(struct bonding
> *bond)
> gso_max_size = min(gso_max_size, slave->dev->gso_max_size);
> gso_max_segs = min(gso_max_segs, slave->dev->gso_max_segs);
> }
> + bond_dev->hard_header_len = max_hard_header_len;
>
> done:
> bond_dev->vlan_features = vlan_features;
> bond_dev->hw_enc_features = enc_features | NETIF_F_GSO_ENCAP_ALL;
> - bond_dev->hard_header_len = max_hard_header_len;
> bond_dev->gso_max_segs = gso_max_segs;
> netif_set_gso_max_size(bond_dev, gso_max_size);
>
> --
> 2.9.3
>
