diff options
author | David S. Miller <davem@davemloft.net> | 2008-09-02 23:14:15 -0400 |
---|---|---|
committer | David S. Miller <davem@davemloft.net> | 2008-09-02 23:14:15 -0400 |
commit | 37b08e34a98c664bea86e3fae718ac45a46b7276 (patch) | |
tree | 8d0cc32bdbfda4d992177aeda56700bea0f918dd | |
parent | 06770843c2f0f929a6e0c758dc433902a01aabfb (diff) |
ipsec: Fix deadlock in xfrm_state management.
Ever since commit 4c563f7669c10a12354b72b518c2287ffc6ebfb3
("[XFRM]: Speed up xfrm_policy and xfrm_state walking") it is
illegal to call __xfrm_state_destroy (and thus xfrm_state_put())
with xfrm_state_lock held. If we do, we'll deadlock since we
have the lock already and __xfrm_state_destroy() tries to take
it again.
Fix this by pushing the xfrm_state_put() calls after the lock
is dropped.
Signed-off-by: David S. Miller <davem@davemloft.net>
-rw-r--r-- | net/xfrm/xfrm_state.c | 32 |
1 files changed, 23 insertions, 9 deletions
diff --git a/net/xfrm/xfrm_state.c b/net/xfrm/xfrm_state.c index 4c6914ef7d92..7bd62f61593f 100644 --- a/net/xfrm/xfrm_state.c +++ b/net/xfrm/xfrm_state.c | |||
@@ -780,11 +780,13 @@ xfrm_state_find(xfrm_address_t *daddr, xfrm_address_t *saddr, | |||
780 | { | 780 | { |
781 | unsigned int h; | 781 | unsigned int h; |
782 | struct hlist_node *entry; | 782 | struct hlist_node *entry; |
783 | struct xfrm_state *x, *x0; | 783 | struct xfrm_state *x, *x0, *to_put; |
784 | int acquire_in_progress = 0; | 784 | int acquire_in_progress = 0; |
785 | int error = 0; | 785 | int error = 0; |
786 | struct xfrm_state *best = NULL; | 786 | struct xfrm_state *best = NULL; |
787 | 787 | ||
788 | to_put = NULL; | ||
789 | |||
788 | spin_lock_bh(&xfrm_state_lock); | 790 | spin_lock_bh(&xfrm_state_lock); |
789 | h = xfrm_dst_hash(daddr, saddr, tmpl->reqid, family); | 791 | h = xfrm_dst_hash(daddr, saddr, tmpl->reqid, family); |
790 | hlist_for_each_entry(x, entry, xfrm_state_bydst+h, bydst) { | 792 | hlist_for_each_entry(x, entry, xfrm_state_bydst+h, bydst) { |
@@ -833,7 +835,7 @@ xfrm_state_find(xfrm_address_t *daddr, xfrm_address_t *saddr, | |||
833 | if (tmpl->id.spi && | 835 | if (tmpl->id.spi && |
834 | (x0 = __xfrm_state_lookup(daddr, tmpl->id.spi, | 836 | (x0 = __xfrm_state_lookup(daddr, tmpl->id.spi, |
835 | tmpl->id.proto, family)) != NULL) { | 837 | tmpl->id.proto, family)) != NULL) { |
836 | xfrm_state_put(x0); | 838 | to_put = x0; |
837 | error = -EEXIST; | 839 | error = -EEXIST; |
838 | goto out; | 840 | goto out; |
839 | } | 841 | } |
@@ -849,7 +851,7 @@ xfrm_state_find(xfrm_address_t *daddr, xfrm_address_t *saddr, | |||
849 | error = security_xfrm_state_alloc_acquire(x, pol->security, fl->secid); | 851 | error = security_xfrm_state_alloc_acquire(x, pol->security, fl->secid); |
850 | if (error) { | 852 | if (error) { |
851 | x->km.state = XFRM_STATE_DEAD; | 853 | x->km.state = XFRM_STATE_DEAD; |
852 | xfrm_state_put(x); | 854 | to_put = x; |
853 | x = NULL; | 855 | x = NULL; |
854 | goto out; | 856 | goto out; |
855 | } | 857 | } |
@@ -870,7 +872,7 @@ xfrm_state_find(xfrm_address_t *daddr, xfrm_address_t *saddr, | |||
870 | xfrm_hash_grow_check(x->bydst.next != NULL); | 872 | xfrm_hash_grow_check(x->bydst.next != NULL); |
871 | } else { | 873 | } else { |
872 | x->km.state = XFRM_STATE_DEAD; | 874 | x->km.state = XFRM_STATE_DEAD; |
873 | xfrm_state_put(x); | 875 | to_put = x; |
874 | x = NULL; | 876 | x = NULL; |
875 | error = -ESRCH; | 877 | error = -ESRCH; |
876 | } | 878 | } |
@@ -881,6 +883,8 @@ out: | |||
881 | else | 883 | else |
882 | *err = acquire_in_progress ? -EAGAIN : error; | 884 | *err = acquire_in_progress ? -EAGAIN : error; |
883 | spin_unlock_bh(&xfrm_state_lock); | 885 | spin_unlock_bh(&xfrm_state_lock); |
886 | if (to_put) | ||
887 | xfrm_state_put(to_put); | ||
884 | return x; | 888 | return x; |
885 | } | 889 | } |
886 | 890 | ||
@@ -1067,18 +1071,20 @@ static struct xfrm_state *__xfrm_find_acq_byseq(u32 seq); | |||
1067 | 1071 | ||
1068 | int xfrm_state_add(struct xfrm_state *x) | 1072 | int xfrm_state_add(struct xfrm_state *x) |
1069 | { | 1073 | { |
1070 | struct xfrm_state *x1; | 1074 | struct xfrm_state *x1, *to_put; |
1071 | int family; | 1075 | int family; |
1072 | int err; | 1076 | int err; |
1073 | int use_spi = xfrm_id_proto_match(x->id.proto, IPSEC_PROTO_ANY); | 1077 | int use_spi = xfrm_id_proto_match(x->id.proto, IPSEC_PROTO_ANY); |
1074 | 1078 | ||
1075 | family = x->props.family; | 1079 | family = x->props.family; |
1076 | 1080 | ||
1081 | to_put = NULL; | ||
1082 | |||
1077 | spin_lock_bh(&xfrm_state_lock); | 1083 | spin_lock_bh(&xfrm_state_lock); |
1078 | 1084 | ||
1079 | x1 = __xfrm_state_locate(x, use_spi, family); | 1085 | x1 = __xfrm_state_locate(x, use_spi, family); |
1080 | if (x1) { | 1086 | if (x1) { |
1081 | xfrm_state_put(x1); | 1087 | to_put = x1; |
1082 | x1 = NULL; | 1088 | x1 = NULL; |
1083 | err = -EEXIST; | 1089 | err = -EEXIST; |
1084 | goto out; | 1090 | goto out; |
@@ -1088,7 +1094,7 @@ int xfrm_state_add(struct xfrm_state *x) | |||
1088 | x1 = __xfrm_find_acq_byseq(x->km.seq); | 1094 | x1 = __xfrm_find_acq_byseq(x->km.seq); |
1089 | if (x1 && ((x1->id.proto != x->id.proto) || | 1095 | if (x1 && ((x1->id.proto != x->id.proto) || |
1090 | xfrm_addr_cmp(&x1->id.daddr, &x->id.daddr, family))) { | 1096 | xfrm_addr_cmp(&x1->id.daddr, &x->id.daddr, family))) { |
1091 | xfrm_state_put(x1); | 1097 | to_put = x1; |
1092 | x1 = NULL; | 1098 | x1 = NULL; |
1093 | } | 1099 | } |
1094 | } | 1100 | } |
@@ -1110,6 +1116,9 @@ out: | |||
1110 | xfrm_state_put(x1); | 1116 | xfrm_state_put(x1); |
1111 | } | 1117 | } |
1112 | 1118 | ||
1119 | if (to_put) | ||
1120 | xfrm_state_put(to_put); | ||
1121 | |||
1113 | return err; | 1122 | return err; |
1114 | } | 1123 | } |
1115 | EXPORT_SYMBOL(xfrm_state_add); | 1124 | EXPORT_SYMBOL(xfrm_state_add); |
@@ -1269,10 +1278,12 @@ EXPORT_SYMBOL(xfrm_state_migrate); | |||
1269 | 1278 | ||
1270 | int xfrm_state_update(struct xfrm_state *x) | 1279 | int xfrm_state_update(struct xfrm_state *x) |
1271 | { | 1280 | { |
1272 | struct xfrm_state *x1; | 1281 | struct xfrm_state *x1, *to_put; |
1273 | int err; | 1282 | int err; |
1274 | int use_spi = xfrm_id_proto_match(x->id.proto, IPSEC_PROTO_ANY); | 1283 | int use_spi = xfrm_id_proto_match(x->id.proto, IPSEC_PROTO_ANY); |
1275 | 1284 | ||
1285 | to_put = NULL; | ||
1286 | |||
1276 | spin_lock_bh(&xfrm_state_lock); | 1287 | spin_lock_bh(&xfrm_state_lock); |
1277 | x1 = __xfrm_state_locate(x, use_spi, x->props.family); | 1288 | x1 = __xfrm_state_locate(x, use_spi, x->props.family); |
1278 | 1289 | ||
@@ -1281,7 +1292,7 @@ int xfrm_state_update(struct xfrm_state *x) | |||
1281 | goto out; | 1292 | goto out; |
1282 | 1293 | ||
1283 | if (xfrm_state_kern(x1)) { | 1294 | if (xfrm_state_kern(x1)) { |
1284 | xfrm_state_put(x1); | 1295 | to_put = x1; |
1285 | err = -EEXIST; | 1296 | err = -EEXIST; |
1286 | goto out; | 1297 | goto out; |
1287 | } | 1298 | } |
@@ -1295,6 +1306,9 @@ int xfrm_state_update(struct xfrm_state *x) | |||
1295 | out: | 1306 | out: |
1296 | spin_unlock_bh(&xfrm_state_lock); | 1307 | spin_unlock_bh(&xfrm_state_lock); |
1297 | 1308 | ||
1309 | if (to_put) | ||
1310 | xfrm_state_put(to_put); | ||
1311 | |||
1298 | if (err) | 1312 | if (err) |
1299 | return err; | 1313 | return err; |
1300 | 1314 | ||