This check is redundant because xfrm_ip2inner_mode no longer returns NULL, because of the change in commit c9500d7b7de8 ("xfrm: store xfrm_mode directly, not its address"). Signed-off-by: Jianbo Liu Reviewed-by: Cosmin Ratiu --- net/ipv4/ip_vti.c | 8 +------- net/ipv6/ip6_vti.c | 8 +------- net/xfrm/xfrm_interface_core.c | 8 +------- net/xfrm/xfrm_policy.c | 9 ++------- 4 files changed, 5 insertions(+), 28 deletions(-) diff --git a/net/ipv4/ip_vti.c b/net/ipv4/ip_vti.c index 95b6bb78fcd2..91e889965918 100644 --- a/net/ipv4/ip_vti.c +++ b/net/ipv4/ip_vti.c @@ -120,14 +120,8 @@ static int vti_rcv_cb(struct sk_buff *skb, int err) inner_mode = &x->inner_mode; - if (x->sel.family == AF_UNSPEC) { + if (x->sel.family == AF_UNSPEC) inner_mode = xfrm_ip2inner_mode(x, XFRM_MODE_SKB_CB(skb)->protocol); - if (inner_mode == NULL) { - XFRM_INC_STATS(dev_net(skb->dev), - LINUX_MIB_XFRMINSTATEMODEERROR); - return -EINVAL; - } - } family = inner_mode->family; diff --git a/net/ipv6/ip6_vti.c b/net/ipv6/ip6_vti.c index ad5290be4dd6..67030918279c 100644 --- a/net/ipv6/ip6_vti.c +++ b/net/ipv6/ip6_vti.c @@ -364,14 +364,8 @@ static int vti6_rcv_cb(struct sk_buff *skb, int err) inner_mode = &x->inner_mode; - if (x->sel.family == AF_UNSPEC) { + if (x->sel.family == AF_UNSPEC) inner_mode = xfrm_ip2inner_mode(x, XFRM_MODE_SKB_CB(skb)->protocol); - if (inner_mode == NULL) { - XFRM_INC_STATS(dev_net(skb->dev), - LINUX_MIB_XFRMINSTATEMODEERROR); - return -EINVAL; - } - } family = inner_mode->family; diff --git a/net/xfrm/xfrm_interface_core.c b/net/xfrm/xfrm_interface_core.c index 330a05286a56..7084c7e6cf7a 100644 --- a/net/xfrm/xfrm_interface_core.c +++ b/net/xfrm/xfrm_interface_core.c @@ -389,14 +389,8 @@ static int xfrmi_rcv_cb(struct sk_buff *skb, int err) if (xnet) { inner_mode = &x->inner_mode; - if (x->sel.family == AF_UNSPEC) { + if (x->sel.family == AF_UNSPEC) inner_mode = xfrm_ip2inner_mode(x, XFRM_MODE_SKB_CB(skb)->protocol); - if (inner_mode == NULL) { - XFRM_INC_STATS(dev_net(skb->dev), - LINUX_MIB_XFRMINSTATEMODEERROR); - return -EINVAL; - } - } if (!xfrm_policy_check(NULL, XFRM_POLICY_IN, skb, inner_mode->family)) diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c index 62486f866975..a1d995db6293 100644 --- a/net/xfrm/xfrm_policy.c +++ b/net/xfrm/xfrm_policy.c @@ -2711,15 +2711,10 @@ static struct dst_entry *xfrm_bundle_create(struct xfrm_policy *policy, */ xfrm_dst_set_child(xdst_prev, &xdst->u.dst); - if (xfrm[i]->sel.family == AF_UNSPEC) { + if (xfrm[i]->sel.family == AF_UNSPEC) inner_mode = xfrm_ip2inner_mode(xfrm[i], xfrm_af2proto(family)); - if (!inner_mode) { - err = -EAFNOSUPPORT; - dst_release(dst); - goto put_states; - } - } else + else inner_mode = &xfrm[i]->inner_mode; xdst->route = dst; -- 2.49.0 In the output path, xfrm_dev_offload_ok and xfrm_get_inner_ipproto need to determine the protocol family of the inner packet (skb) before it gets encapsulated. In xfrm_dev_offload_ok, the code checked x->inner_mode.family. This is incorrect because the state's true inner family might be specified in x->inner_mode_iaf.family (e.g., for tunnel mode). In xfrm_get_inner_ipproto, the code checked x->outer_mode.family. This is also incorrect for tunnel mode, as the inner packet's family can be different from the outer header's family. At both of these call sites, the skb variable holds the original inner packet. The most direct and reliable source of truth for its protocol family is its destination entry. This patch fixes the issue by using skb_dst(skb)->ops->family to ensure protocol-specific headers are only accessed for the correct packet type. Signed-off-by: Jianbo Liu Reviewed-by: Cosmin Ratiu --- net/xfrm/xfrm_device.c | 2 +- net/xfrm/xfrm_output.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/net/xfrm/xfrm_device.c b/net/xfrm/xfrm_device.c index 44b9de6e4e77..52ae0e034d29 100644 --- a/net/xfrm/xfrm_device.c +++ b/net/xfrm/xfrm_device.c @@ -438,7 +438,7 @@ bool xfrm_dev_offload_ok(struct sk_buff *skb, struct xfrm_state *x) check_tunnel_size = x->xso.type == XFRM_DEV_OFFLOAD_PACKET && x->props.mode == XFRM_MODE_TUNNEL; - switch (x->inner_mode.family) { + switch (skb_dst(skb)->ops->family) { case AF_INET: /* Check for IPv4 options */ if (ip_hdr(skb)->ihl != 5) diff --git a/net/xfrm/xfrm_output.c b/net/xfrm/xfrm_output.c index 9077730ff7d0..a98b5bf55ac3 100644 --- a/net/xfrm/xfrm_output.c +++ b/net/xfrm/xfrm_output.c @@ -698,7 +698,7 @@ static void xfrm_get_inner_ipproto(struct sk_buff *skb, struct xfrm_state *x) return; if (x->outer_mode.encap == XFRM_MODE_TUNNEL) { - switch (x->outer_mode.family) { + switch (skb_dst(skb)->ops->family) { case AF_INET: xo->inner_ipproto = ip_hdr(skb)->protocol; break; -- 2.49.0 The GSO segmentation functions for ESP tunnel mode (xfrm4_tunnel_gso_segment and xfrm6_tunnel_gso_segment) were determining the inner packet's L2 protocol type by checking the static x->inner_mode.family field from the xfrm state. This is unreliable. In tunnel mode, the state's actual inner family could be defined by x->inner_mode.family or by x->inner_mode_iaf.family. Checking only the former can lead to a mismatch with the actual packet being processed, causing GSO to create segments with the wrong L2 header type. This patch fixes the bug by deriving the inner mode directly from the packet's inner protocol stored in XFRM_MODE_SKB_CB(skb)->protocol. Signed-off-by: Jianbo Liu Reviewed-by: Cosmin Ratiu --- net/ipv4/esp4_offload.c | 6 ++++-- net/ipv6/esp6_offload.c | 6 ++++-- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/net/ipv4/esp4_offload.c b/net/ipv4/esp4_offload.c index e0d94270da28..05828d4cb6cd 100644 --- a/net/ipv4/esp4_offload.c +++ b/net/ipv4/esp4_offload.c @@ -122,8 +122,10 @@ static struct sk_buff *xfrm4_tunnel_gso_segment(struct xfrm_state *x, struct sk_buff *skb, netdev_features_t features) { - __be16 type = x->inner_mode.family == AF_INET6 ? htons(ETH_P_IPV6) - : htons(ETH_P_IP); + const struct xfrm_mode *inner_mode = xfrm_ip2inner_mode(x, + XFRM_MODE_SKB_CB(skb)->protocol); + __be16 type = inner_mode->family == AF_INET6 ? htons(ETH_P_IPV6) + : htons(ETH_P_IP); return skb_eth_gso_segment(skb, features, type); } diff --git a/net/ipv6/esp6_offload.c b/net/ipv6/esp6_offload.c index 7b41fb4f00b5..22410243ebe8 100644 --- a/net/ipv6/esp6_offload.c +++ b/net/ipv6/esp6_offload.c @@ -158,8 +158,10 @@ static struct sk_buff *xfrm6_tunnel_gso_segment(struct xfrm_state *x, struct sk_buff *skb, netdev_features_t features) { - __be16 type = x->inner_mode.family == AF_INET ? htons(ETH_P_IP) - : htons(ETH_P_IPV6); + const struct xfrm_mode *inner_mode = xfrm_ip2inner_mode(x, + XFRM_MODE_SKB_CB(skb)->protocol); + __be16 type = inner_mode->family == AF_INET ? htons(ETH_P_IP) + : htons(ETH_P_IPV6); return skb_eth_gso_segment(skb, features, type); } -- 2.49.0