Skip to content

Commit fa90a31

Browse files
Cosmin Ratiuklassert
authored andcommitted
xfrm: Don't clobber inner headers when already set
On VXLAN over IPsec egress, xfrm{4,6}_transport_output() blindly overwrite inner_transport_header (== the inner TCP header saved in VXLAN iptunnel_handle_offloads() -> skb_reset_inner_headers()) with the current transport_header (== the VXLAN outer UDP header set by udp_tunnel_xmit_skb()). This was a latent bug, harmless until commit [1] added a doff validation check in qdisc_pkt_len_segs_init() for encapsulated GSO packets. With the wrong inner_transport_header set by xfrm, qdisc_pkt_len_segs_init() interprets inner_transport_header as a TCP header, reads doff=0 from the upper byte of the VNI and drops the packet with DROP_REASON_SKB_BAD_GSO. Besides the use in GSO to determine the header size of segmented packets, inner_transport_header might be used by drivers to set up inner checksum offloading by pointing the HW to the inner transport header. A quick browse through available drivers shows that mlx5 uses skb->csum_start specifically for this scenario, while others either don't support VXLAN over IPsec crypto offload (ixgbe) or the HW is capable of parsing the packets itself (nfp, Chelsio). But in all cases, it is more correct to let the inner_transport_header point to the innermost header instead of overwriting it in xfrm. So fix this by guarding all four inner header save sites in xfrm_output.c (xfrm{4,6}_transport_output, xfrm{4,6}_tunnel_encap_add) with a check for skb->inner_protocol. When inner_protocol is set, a tunnel layer (VXLAN, Geneve, GRE, etc.) has already saved the correct inner header offsets and they must not be overwritten. When inner_protocol is zero, no prior tunnel encapsulation exists and xfrm must save the inner headers itself. The tunnel mode checks are only added for completion, since they aren't strictly required, as xfrm_output() forces software GSO in tunnel mode before encap. This makes the previously added test pass: # ./tools/testing/selftests/drivers/net/hw/ipsec_vxlan.py TAP version 13 1..4 ok 1 ipsec_vxlan.test_vxlan_ipsec_crypto_offload.outer_v4_inner_v4 ok 2 ipsec_vxlan.test_vxlan_ipsec_crypto_offload.outer_v4_inner_v6 ok 3 ipsec_vxlan.test_vxlan_ipsec_crypto_offload.outer_v6_inner_v4 ok 4 ipsec_vxlan.test_vxlan_ipsec_crypto_offload.outer_v6_inner_v6 # Totals: pass:4 fail:0 xfail:0 xpass:0 skip:0 error:0 [1] commit 7fb4c19 ("net: pull headers in qdisc_pkt_len_segs_init()") Fixes: f1bd7d6 ("xfrm: Add encapsulation header offsets while SKB is not encrypted") Signed-off-by: Cosmin Ratiu <cratiu@nvidia.com> Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
1 parent e64e03b commit fa90a31

1 file changed

Lines changed: 14 additions & 6 deletions

File tree

net/xfrm/xfrm_output.c

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,9 @@ static int xfrm4_transport_output(struct xfrm_state *x, struct sk_buff *skb)
6666
struct iphdr *iph = ip_hdr(skb);
6767
int ihl = iph->ihl * 4;
6868

69-
skb_set_inner_transport_header(skb, skb_transport_offset(skb));
69+
if (!skb->inner_protocol)
70+
skb_set_inner_transport_header(skb,
71+
skb_transport_offset(skb));
7072

7173
skb_set_network_header(skb, -x->props.header_len);
7274
skb->mac_header = skb->network_header +
@@ -167,7 +169,9 @@ static int xfrm6_transport_output(struct xfrm_state *x, struct sk_buff *skb)
167169
int hdr_len;
168170

169171
iph = ipv6_hdr(skb);
170-
skb_set_inner_transport_header(skb, skb_transport_offset(skb));
172+
if (!skb->inner_protocol)
173+
skb_set_inner_transport_header(skb,
174+
skb_transport_offset(skb));
171175

172176
hdr_len = xfrm6_hdr_offset(x, skb, &prevhdr);
173177
if (hdr_len < 0)
@@ -276,8 +280,10 @@ static int xfrm4_tunnel_encap_add(struct xfrm_state *x, struct sk_buff *skb)
276280
struct iphdr *top_iph;
277281
int flags;
278282

279-
skb_set_inner_network_header(skb, skb_network_offset(skb));
280-
skb_set_inner_transport_header(skb, skb_transport_offset(skb));
283+
if (!skb->inner_protocol) {
284+
skb_set_inner_network_header(skb, skb_network_offset(skb));
285+
skb_set_inner_transport_header(skb, skb_transport_offset(skb));
286+
}
281287

282288
skb_set_network_header(skb, -x->props.header_len);
283289
skb->mac_header = skb->network_header +
@@ -321,8 +327,10 @@ static int xfrm6_tunnel_encap_add(struct xfrm_state *x, struct sk_buff *skb)
321327
struct ipv6hdr *top_iph;
322328
int dsfield;
323329

324-
skb_set_inner_network_header(skb, skb_network_offset(skb));
325-
skb_set_inner_transport_header(skb, skb_transport_offset(skb));
330+
if (!skb->inner_protocol) {
331+
skb_set_inner_network_header(skb, skb_network_offset(skb));
332+
skb_set_inner_transport_header(skb, skb_transport_offset(skb));
333+
}
326334

327335
skb_set_network_header(skb, -x->props.header_len);
328336
skb->mac_header = skb->network_header +

0 commit comments

Comments
 (0)