* [PATCH 1/3] xfrm: Fix xfrm_dst_cache memleak
2017-11-01 10:05 pull request (net): ipsec 2017-11-01 Steffen Klassert
@ 2017-11-01 10:05 ` Steffen Klassert
2017-11-01 10:05 ` [PATCH 2/3] xfrm: Clear sk_dst_cache when applying per-socket policy Steffen Klassert
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Steffen Klassert @ 2017-11-01 10:05 UTC (permalink / raw)
To: David Miller; +Cc: Herbert Xu, Steffen Klassert, netdev
We have a memleak whenever a flow matches a policy without
a matching SA. In this case we generate a dummy bundle and
take an additional refcount on the dst_entry. This was needed
as long as we had the flowcache. The flowcache removal patches
deleted all related refcounts but forgot the one for the
dummy bundle case. Fix the memleak by removing this refcount.
Fixes: 3ca28286ea80 ("xfrm_policy: bypass flow_cache_lookup")
Reported-by: Maxime Bizon <mbizon@freebox.fr>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
net/xfrm/xfrm_policy.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c
index 2746b62..8cafb3c 100644
--- a/net/xfrm/xfrm_policy.c
+++ b/net/xfrm/xfrm_policy.c
@@ -2076,7 +2076,6 @@ xfrm_bundle_lookup(struct net *net, const struct flowi *fl, u16 family, u8 dir,
xdst->num_xfrms = num_xfrms;
memcpy(xdst->pols, pols, sizeof(struct xfrm_policy *) * num_pols);
- dst_hold(&xdst->u.dst);
return xdst;
inc_error:
--
2.7.4
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH 2/3] xfrm: Clear sk_dst_cache when applying per-socket policy.
2017-11-01 10:05 pull request (net): ipsec 2017-11-01 Steffen Klassert
2017-11-01 10:05 ` [PATCH 1/3] xfrm: Fix xfrm_dst_cache memleak Steffen Klassert
@ 2017-11-01 10:05 ` Steffen Klassert
2017-11-01 10:05 ` [PATCH 3/3] xfrm: Fix GSO for IPsec with GRE tunnel Steffen Klassert
2017-11-01 10:32 ` pull request (net): ipsec 2017-11-01 David Miller
3 siblings, 0 replies; 5+ messages in thread
From: Steffen Klassert @ 2017-11-01 10:05 UTC (permalink / raw)
To: David Miller; +Cc: Herbert Xu, Steffen Klassert, netdev
From: Jonathan Basseri <misterikkit@google.com>
If a socket has a valid dst cache, then xfrm_lookup_route will get
skipped. However, the cache is not invalidated when applying policy to a
socket (i.e. IPV6_XFRM_POLICY). The result is that new policies are
sometimes ignored on those sockets. (Note: This was broken for IPv4 and
IPv6 at different times.)
This can be demonstrated like so,
1. Create UDP socket.
2. connect() the socket.
3. Apply an outbound XFRM policy to the socket. (setsockopt)
4. send() data on the socket.
Packets will continue to be sent in the clear instead of matching an
xfrm or returning a no-match error (EAGAIN). This affects calls to
send() and not sendto().
Invalidating the sk_dst_cache is necessary to correctly apply xfrm
policies. Since we do this in xfrm_user_policy(), the sk_lock was
already acquired in either do_ip_setsockopt() or do_ipv6_setsockopt(),
and we may call __sk_dst_reset().
Performance impact should be negligible, since this code is only called
when changing xfrm policy, and only affects the socket in question.
Fixes: 00bc0ef5880d ("ipv6: Skip XFRM lookup if dst_entry in socket cache is valid")
Tested: https://android-review.googlesource.com/517555
Tested: https://android-review.googlesource.com/418659
Signed-off-by: Jonathan Basseri <misterikkit@google.com>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
net/xfrm/xfrm_state.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/net/xfrm/xfrm_state.c b/net/xfrm/xfrm_state.c
index 1221347..1f5cee2 100644
--- a/net/xfrm/xfrm_state.c
+++ b/net/xfrm/xfrm_state.c
@@ -2069,6 +2069,7 @@ int xfrm_user_policy(struct sock *sk, int optname, u8 __user *optval, int optlen
if (err >= 0) {
xfrm_sk_policy_insert(sk, err, pol);
xfrm_pol_put(pol);
+ __sk_dst_reset(sk);
err = 0;
}
--
2.7.4
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH 3/3] xfrm: Fix GSO for IPsec with GRE tunnel.
2017-11-01 10:05 pull request (net): ipsec 2017-11-01 Steffen Klassert
2017-11-01 10:05 ` [PATCH 1/3] xfrm: Fix xfrm_dst_cache memleak Steffen Klassert
2017-11-01 10:05 ` [PATCH 2/3] xfrm: Clear sk_dst_cache when applying per-socket policy Steffen Klassert
@ 2017-11-01 10:05 ` Steffen Klassert
2017-11-01 10:32 ` pull request (net): ipsec 2017-11-01 David Miller
3 siblings, 0 replies; 5+ messages in thread
From: Steffen Klassert @ 2017-11-01 10:05 UTC (permalink / raw)
To: David Miller; +Cc: Herbert Xu, Steffen Klassert, netdev
We reset the encapsulation field of the skb too early
in xfrm_output. As a result, the GRE GSO handler does
not segment the packets. This leads to a performance
drop down. We fix this by resetting the encapsulation
field right before we do the transformation, when
the inner headers become invalid.
Fixes: f1bd7d659ef0 ("xfrm: Add encapsulation header offsets while SKB is not encrypted")
Reported-by: Vicente De Luca <vdeluca@zendesk.com>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
net/xfrm/xfrm_output.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/net/xfrm/xfrm_output.c b/net/xfrm/xfrm_output.c
index 31a2e6d..73ad8c8 100644
--- a/net/xfrm/xfrm_output.c
+++ b/net/xfrm/xfrm_output.c
@@ -105,6 +105,9 @@ static int xfrm_output_one(struct sk_buff *skb, int err)
if (xfrm_offload(skb)) {
x->type_offload->encap(x, skb);
} else {
+ /* Inner headers are invalid now. */
+ skb->encapsulation = 0;
+
err = x->type->output(x, skb);
if (err == -EINPROGRESS)
goto out;
@@ -208,7 +211,6 @@ int xfrm_output(struct sock *sk, struct sk_buff *skb)
int err;
secpath_reset(skb);
- skb->encapsulation = 0;
if (xfrm_dev_offload_ok(skb, x)) {
struct sec_path *sp;
--
2.7.4
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: pull request (net): ipsec 2017-11-01
2017-11-01 10:05 pull request (net): ipsec 2017-11-01 Steffen Klassert
` (2 preceding siblings ...)
2017-11-01 10:05 ` [PATCH 3/3] xfrm: Fix GSO for IPsec with GRE tunnel Steffen Klassert
@ 2017-11-01 10:32 ` David Miller
3 siblings, 0 replies; 5+ messages in thread
From: David Miller @ 2017-11-01 10:32 UTC (permalink / raw)
To: steffen.klassert; +Cc: herbert, netdev
From: Steffen Klassert <steffen.klassert@secunet.com>
Date: Wed, 1 Nov 2017 11:05:00 +0100
> 1) Fix a memleak when a packet matches a policy
> without a matching state.
>
> 2) Reset the socket cached dst_entry when inserting
> a socket policy, otherwise the policy might be
> ignored. From Jonathan Basseri.
>
> 3) Fix GSO for a IPsec, GRE tunnel combination.
> We reset the encapsulation field at the skb
> too erly, as a result GRE does not segment
> GSO packets. Fix this by resetting the the
> encapsulation field right before the
> transformation where the inner headers get
> invalid.
>
> Please pull or let me know if there are problems.
Pulled, thanks.
^ permalink raw reply [flat|nested] 5+ messages in thread