Skip to contentSkip to conversation

xfrm: Refcount destination entry on xfrm_lookup

PATCH In review Steffen Klassert 3 participants 3 messages Mar 16, 2011

Subsystems (3)
lore
Share & tools
lore

Steffen Klassert

author maintainer
#0 lore raw

PATCH 1/1 xfrm: Refcount destination entry on xfrm_lookup

We return a destination entry without refcount if a socket policy is found in xfrm_lookup. This triggers a warning on a negative refcount when freeeing this dst entry. So take a refcount in this case to fix it.

This refcount was forgotten when xfrm changed to cache bundles instead of policies for outgoing flows.

---
 net/xfrm/xfrm_policy.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)
net/xfrm/xfrm_policy.c +2 -0
index 027e3c6..15792d8 100644
@@ -1804,6 +1804,8 @@ restart:
 				goto no_transform;
 			}
 
+			dst_hold(&xdst->u.dst);
+
 			spin_lock_bh(&xfrm_policy_sk_bundle_lock);
 			xdst->u.dst.next = xfrm_policy_sk_bundles;
 			xfrm_policy_sk_bundles = &xdst->u.dst;
Signature · 1 line
1.7.0.4
Acked-by on v1 1/1
#1 lore raw
Quoted 9 lines · Steffen Klassert

We return a destination entry without refcount if a socket policy is found in xfrm_lookup. This triggers a warning on a negative refcount when freeeing this dst entry. So take a refcount in this case to fix it.

This refcount was forgotten when xfrm changed to cache bundles instead of policies for outgoing flows.

Ok. This is one of the corner cases I did not test: having socket policy something else than no action. I just didn't have the application ready for it. Sorry about the bug. And good catch Steffen.

Quoted 17 lines
---
 net/xfrm/xfrm_policy.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)
net/xfrm/xfrm_policy.c +2 -0
index 027e3c6..15792d8 100644
@@ -1804,6 +1804,8 @@ restart:
 				goto no_transform;
 			}
 
+			dst_hold(&xdst->u.dst);
+
 			spin_lock_bh(&xfrm_policy_sk_bundle_lock);
 			xdst->u.dst.next = xfrm_policy_sk_bundles;
 			xfrm_policy_sk_bundles = &xdst->u.dst;
maintainer
#2 lore raw

From: Timo Teräs <[email protected]>
Date: Wed, 16 Mar 2011 10:03:23 +0200

Quoted 9 lines · Steffen Klassert
Quoted 8 lines

We return a destination entry without refcount if a socket policy is found in xfrm_lookup. This triggers a warning on a negative refcount when freeeing this dst entry. So take a refcount in this case to fix it.

This refcount was forgotten when xfrm changed to cache bundles instead of policies for outgoing flows.

Quoted 7 lines
Quoted 1 line

Ok. This is one of the corner cases I did not test: having socket policy something else than no action. I just didn't have the application ready for it. Sorry about the bug. And good catch Steffen.

Applied and queued up for -stable, thanks!