Commit ea2dea9d authored by Timo Teräs's avatar Timo Teräs Committed by David S. Miller

xfrm: remove policy lock when accessing policy->walk.dead

All of the code considers ->dead as a hint that the cached policy
needs to get refreshed. The read side can just drop the read lock
without any side effects.

The write side needs to make sure that it's written only exactly
once. Only possible race is at xfrm_policy_kill(). This is fixed
by checking result of __xfrm_policy_unlink() when needed. It will
always succeed if the policy object is looked up from the hash
list (so some checks are removed), but it needs to be checked if
we are trying to unlink policy via a reference (appropriate
checks added).

Since policy->walk.dead is written exactly once, it no longer
needs to be protected with a write lock.
Signed-off-by: default avatarTimo Teras <timo.teras@iki.fi>
Acked-by: default avatarHerbert Xu <herbert@gondor.apana.org.au>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parent c8bf4d04
...@@ -156,7 +156,7 @@ static void xfrm_policy_timer(unsigned long data) ...@@ -156,7 +156,7 @@ static void xfrm_policy_timer(unsigned long data)
read_lock(&xp->lock); read_lock(&xp->lock);
if (xp->walk.dead) if (unlikely(xp->walk.dead))
goto out; goto out;
dir = xfrm_policy_id2dir(xp->index); dir = xfrm_policy_id2dir(xp->index);
...@@ -297,17 +297,7 @@ static DECLARE_WORK(xfrm_policy_gc_work, xfrm_policy_gc_task); ...@@ -297,17 +297,7 @@ static DECLARE_WORK(xfrm_policy_gc_work, xfrm_policy_gc_task);
static void xfrm_policy_kill(struct xfrm_policy *policy) static void xfrm_policy_kill(struct xfrm_policy *policy)
{ {
int dead;
write_lock_bh(&policy->lock);
dead = policy->walk.dead;
policy->walk.dead = 1; policy->walk.dead = 1;
write_unlock_bh(&policy->lock);
if (unlikely(dead)) {
WARN_ON(1);
return;
}
spin_lock_bh(&xfrm_policy_gc_lock); spin_lock_bh(&xfrm_policy_gc_lock);
hlist_add_head(&policy->bydst, &xfrm_policy_gc_list); hlist_add_head(&policy->bydst, &xfrm_policy_gc_list);
...@@ -776,7 +766,6 @@ xfrm_policy_flush_secctx_check(struct net *net, u8 type, struct xfrm_audit *audi ...@@ -776,7 +766,6 @@ xfrm_policy_flush_secctx_check(struct net *net, u8 type, struct xfrm_audit *audi
int xfrm_policy_flush(struct net *net, u8 type, struct xfrm_audit *audit_info) int xfrm_policy_flush(struct net *net, u8 type, struct xfrm_audit *audit_info)
{ {
int dir, err = 0, cnt = 0; int dir, err = 0, cnt = 0;
struct xfrm_policy *dp;
write_lock_bh(&xfrm_policy_lock); write_lock_bh(&xfrm_policy_lock);
...@@ -794,10 +783,9 @@ int xfrm_policy_flush(struct net *net, u8 type, struct xfrm_audit *audit_info) ...@@ -794,10 +783,9 @@ int xfrm_policy_flush(struct net *net, u8 type, struct xfrm_audit *audit_info)
&net->xfrm.policy_inexact[dir], bydst) { &net->xfrm.policy_inexact[dir], bydst) {
if (pol->type != type) if (pol->type != type)
continue; continue;
dp = __xfrm_policy_unlink(pol, dir); __xfrm_policy_unlink(pol, dir);
write_unlock_bh(&xfrm_policy_lock); write_unlock_bh(&xfrm_policy_lock);
if (dp) cnt++;
cnt++;
xfrm_audit_policy_delete(pol, 1, audit_info->loginuid, xfrm_audit_policy_delete(pol, 1, audit_info->loginuid,
audit_info->sessionid, audit_info->sessionid,
...@@ -816,10 +804,9 @@ int xfrm_policy_flush(struct net *net, u8 type, struct xfrm_audit *audit_info) ...@@ -816,10 +804,9 @@ int xfrm_policy_flush(struct net *net, u8 type, struct xfrm_audit *audit_info)
bydst) { bydst) {
if (pol->type != type) if (pol->type != type)
continue; continue;
dp = __xfrm_policy_unlink(pol, dir); __xfrm_policy_unlink(pol, dir);
write_unlock_bh(&xfrm_policy_lock); write_unlock_bh(&xfrm_policy_lock);
if (dp) cnt++;
cnt++;
xfrm_audit_policy_delete(pol, 1, xfrm_audit_policy_delete(pol, 1,
audit_info->loginuid, audit_info->loginuid,
...@@ -1132,6 +1119,9 @@ int xfrm_sk_policy_insert(struct sock *sk, int dir, struct xfrm_policy *pol) ...@@ -1132,6 +1119,9 @@ int xfrm_sk_policy_insert(struct sock *sk, int dir, struct xfrm_policy *pol)
__xfrm_policy_link(pol, XFRM_POLICY_MAX+dir); __xfrm_policy_link(pol, XFRM_POLICY_MAX+dir);
} }
if (old_pol) if (old_pol)
/* Unlinking succeeds always. This is the only function
* allowed to delete or replace socket policy.
*/
__xfrm_policy_unlink(old_pol, XFRM_POLICY_MAX+dir); __xfrm_policy_unlink(old_pol, XFRM_POLICY_MAX+dir);
write_unlock_bh(&xfrm_policy_lock); write_unlock_bh(&xfrm_policy_lock);
...@@ -1737,11 +1727,8 @@ int __xfrm_lookup(struct net *net, struct dst_entry **dst_p, struct flowi *fl, ...@@ -1737,11 +1727,8 @@ int __xfrm_lookup(struct net *net, struct dst_entry **dst_p, struct flowi *fl,
goto error; goto error;
} }
for (pi = 0; pi < npols; pi++) { for (pi = 0; pi < npols; pi++)
read_lock_bh(&pols[pi]->lock);
pol_dead |= pols[pi]->walk.dead; pol_dead |= pols[pi]->walk.dead;
read_unlock_bh(&pols[pi]->lock);
}
write_lock_bh(&policy->lock); write_lock_bh(&policy->lock);
if (unlikely(pol_dead || stale_bundle(dst))) { if (unlikely(pol_dead || stale_bundle(dst))) {
......
...@@ -1770,13 +1770,9 @@ static int xfrm_add_pol_expire(struct sk_buff *skb, struct nlmsghdr *nlh, ...@@ -1770,13 +1770,9 @@ static int xfrm_add_pol_expire(struct sk_buff *skb, struct nlmsghdr *nlh,
if (xp == NULL) if (xp == NULL)
return -ENOENT; return -ENOENT;
read_lock(&xp->lock); if (unlikely(xp->walk.dead))
if (xp->walk.dead) {
read_unlock(&xp->lock);
goto out; goto out;
}
read_unlock(&xp->lock);
err = 0; err = 0;
if (up->hard) { if (up->hard) {
uid_t loginuid = NETLINK_CB(skb).loginuid; uid_t loginuid = NETLINK_CB(skb).loginuid;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment