On Thu, 2018-01-25 at 15:31 +1100, Daniel Axtens wrote: > There are a few ways we can send packets that are too large to a > network driver. > > When non-GSO packets are forwarded, we validate their size, based on > the MTU of the destination device. However, when GSO packets are > forwarded, we do not validate their size. We implicitly assume that > when they are segmented, the resultant packets will be correctly > sized. > > This is not always the case. > > We observed a case where a packet received on an ibmveth device had a > GSO size of around 10kB. This was forwarded by Open vSwitch to a bnx2x > device, where it caused a firmware assert. This is described in detail > at [0] and was the genesis of this series. > > Rather than fixing this in the driver, this series fixes the > core path. It does it in 2 steps: > > 1) make is_skb_forwardable check GSO packets - this catches bridges > > 2) make validate_xmit_skb check the size of all packets, so as to > catch everything else (e.g. macvlan, tc mired, OVS) > > I am a bit nervous about how this series will interact with nested > VLANs, as the existing code only allows for one VLAN_HLEN. (Previously > these packets would sail past unchecked.) But I thought it would be > prudent to get more eyes on this sooner rather than later. > > Thanks, > Daniel > > v1: https://www.spinics.net/lists/netdev/msg478634.html > Changes in v2: > > - improve names, thanks Marcelo Ricardo Leitner > > - add check to xmit_validate_skb; thanks to everyone who participated > in the discussion. > > - drop extra check in Open vSwitch. Bad packets will be caught by > validate_xmit_skb for now and we can come back and add it later if > OVS people would like the extra logging. > > [0]: https://patchwork.ozlabs.org/patch/859410/ > > Cc: Jason Wang <jasow...@redhat.com> > Cc: Pravin Shelar <pshe...@ovn.org> > Cc: Marcelo Ricardo Leitner <marcelo.leit...@gmail.com> > Cc: manish.cho...@cavium.com > Cc: d...@openvswitch.org > > Daniel Axtens (4): > net: rename skb_gso_validate_mtu -> skb_gso_validate_network_len > net: move skb_gso_mac_seglen to skbuff.h > net: is_skb_forwardable: check the size of GSO segments > net: check the size of a packet in validate_xmit_skb > > include/linux/skbuff.h | 18 ++++++++- > net/core/dev.c | 24 ++++++++---- > net/core/skbuff.c | 66 > ++++++++++++++++++++++++++------- > net/ipv4/ip_forward.c | 2 +- > net/ipv4/ip_output.c | 2 +- > net/ipv4/netfilter/nf_flow_table_ipv4.c | 2 +- > net/ipv6/ip6_output.c | 2 +- > net/ipv6/netfilter/nf_flow_table_ipv6.c | 2 +- > net/mpls/af_mpls.c | 2 +- > net/sched/sch_tbf.c | 10 ----- > net/xfrm/xfrm_device.c | 2 +- > 11 files changed, 93 insertions(+), 39 deletions(-) >
May I ask which tree are you targeting ? ( Documentation/networking/netdev-FAQ.txt ) Anything touching GSO is very risky and should target net-next, especially considering 4.15 is released this week end. Are we really willing to backport this intrusive series in stable trees, or do we have a smaller fix for bnx2x ? Thanks.
