aboutsummaryrefslogtreecommitdiffstats
path: root/net/xfrm
diff options
context:
space:
mode:
authorEric Paris <eparis@redhat.com>2007-03-07 18:37:58 -0500
committerDavid S. Miller <davem@sunset.davemloft.net>2007-03-07 19:08:09 -0500
commitef41aaa0b755f479012341ac11db9ca5b8928d98 (patch)
treef5cd83b9117d0092f40006fbf4fd1f39652ad925 /net/xfrm
parent05e52dd7396514648fba6c275eb7b49eca333c6d (diff)
[IPSEC]: xfrm_policy delete security check misplaced
The security hooks to check permissions to remove an xfrm_policy were actually done after the policy was removed. Since the unlinking and deletion are done in xfrm_policy_by* functions this moves the hooks inside those 2 functions. There we have all the information needed to do the security check and it can be done before the deletion. Since auditing requires the result of that security check err has to be passed back and forth from the xfrm_policy_by* functions. This patch also fixes a bug where a deletion that failed the security check could cause improper accounting on the xfrm_policy (xfrm_get_policy didn't have a put on the exit path for the hold taken by xfrm_policy_by*) It also fixes the return code when no policy is found in xfrm_add_pol_expire. In old code (at least back in the 2.6.18 days) err wasn't used before the return when no policy is found and so the initialization would cause err to be ENOENT. But since err has since been used above when we don't get a policy back from the xfrm_policy_by* function we would always return 0 instead of the intended ENOENT. Also fixed some white space damage in the same area. Signed-off-by: Eric Paris <eparis@redhat.com> Acked-by: Venkat Yekkirala <vyekkirala@trustedcs.com> Acked-by: James Morris <jmorris@namei.org> Signed-off-by: David S. Miller <davem@davemloft.net>
Diffstat (limited to 'net/xfrm')
-rw-r--r--net/xfrm/xfrm_policy.c18
-rw-r--r--net/xfrm/xfrm_user.c19
2 files changed, 25 insertions, 12 deletions
diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c
index 946b715db5ec..0c3a70ac5075 100644
--- a/net/xfrm/xfrm_policy.c
+++ b/net/xfrm/xfrm_policy.c
@@ -735,12 +735,14 @@ EXPORT_SYMBOL(xfrm_policy_insert);
735 735
736struct xfrm_policy *xfrm_policy_bysel_ctx(u8 type, int dir, 736struct xfrm_policy *xfrm_policy_bysel_ctx(u8 type, int dir,
737 struct xfrm_selector *sel, 737 struct xfrm_selector *sel,
738 struct xfrm_sec_ctx *ctx, int delete) 738 struct xfrm_sec_ctx *ctx, int delete,
739 int *err)
739{ 740{
740 struct xfrm_policy *pol, *ret; 741 struct xfrm_policy *pol, *ret;
741 struct hlist_head *chain; 742 struct hlist_head *chain;
742 struct hlist_node *entry; 743 struct hlist_node *entry;
743 744
745 *err = 0;
744 write_lock_bh(&xfrm_policy_lock); 746 write_lock_bh(&xfrm_policy_lock);
745 chain = policy_hash_bysel(sel, sel->family, dir); 747 chain = policy_hash_bysel(sel, sel->family, dir);
746 ret = NULL; 748 ret = NULL;
@@ -750,6 +752,11 @@ struct xfrm_policy *xfrm_policy_bysel_ctx(u8 type, int dir,
750 xfrm_sec_ctx_match(ctx, pol->security)) { 752 xfrm_sec_ctx_match(ctx, pol->security)) {
751 xfrm_pol_hold(pol); 753 xfrm_pol_hold(pol);
752 if (delete) { 754 if (delete) {
755 *err = security_xfrm_policy_delete(pol);
756 if (*err) {
757 write_unlock_bh(&xfrm_policy_lock);
758 return pol;
759 }
753 hlist_del(&pol->bydst); 760 hlist_del(&pol->bydst);
754 hlist_del(&pol->byidx); 761 hlist_del(&pol->byidx);
755 xfrm_policy_count[dir]--; 762 xfrm_policy_count[dir]--;
@@ -768,12 +775,14 @@ struct xfrm_policy *xfrm_policy_bysel_ctx(u8 type, int dir,
768} 775}
769EXPORT_SYMBOL(xfrm_policy_bysel_ctx); 776EXPORT_SYMBOL(xfrm_policy_bysel_ctx);
770 777
771struct xfrm_policy *xfrm_policy_byid(u8 type, int dir, u32 id, int delete) 778struct xfrm_policy *xfrm_policy_byid(u8 type, int dir, u32 id, int delete,
779 int *err)
772{ 780{
773 struct xfrm_policy *pol, *ret; 781 struct xfrm_policy *pol, *ret;
774 struct hlist_head *chain; 782 struct hlist_head *chain;
775 struct hlist_node *entry; 783 struct hlist_node *entry;
776 784
785 *err = 0;
777 write_lock_bh(&xfrm_policy_lock); 786 write_lock_bh(&xfrm_policy_lock);
778 chain = xfrm_policy_byidx + idx_hash(id); 787 chain = xfrm_policy_byidx + idx_hash(id);
779 ret = NULL; 788 ret = NULL;
@@ -781,6 +790,11 @@ struct xfrm_policy *xfrm_policy_byid(u8 type, int dir, u32 id, int delete)
781 if (pol->type == type && pol->index == id) { 790 if (pol->type == type && pol->index == id) {
782 xfrm_pol_hold(pol); 791 xfrm_pol_hold(pol);
783 if (delete) { 792 if (delete) {
793 *err = security_xfrm_policy_delete(pol);
794 if (*err) {
795 write_unlock_bh(&xfrm_policy_lock);
796 return pol;
797 }
784 hlist_del(&pol->bydst); 798 hlist_del(&pol->bydst);
785 hlist_del(&pol->byidx); 799 hlist_del(&pol->byidx);
786 xfrm_policy_count[dir]--; 800 xfrm_policy_count[dir]--;
diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c
index 956cfe0ff7f8..30c244bbd8ac 100644
--- a/net/xfrm/xfrm_user.c
+++ b/net/xfrm/xfrm_user.c
@@ -1254,7 +1254,7 @@ static int xfrm_get_policy(struct sk_buff *skb, struct nlmsghdr *nlh,
1254 return err; 1254 return err;
1255 1255
1256 if (p->index) 1256 if (p->index)
1257 xp = xfrm_policy_byid(type, p->dir, p->index, delete); 1257 xp = xfrm_policy_byid(type, p->dir, p->index, delete, &err);
1258 else { 1258 else {
1259 struct rtattr *rt = xfrma[XFRMA_SEC_CTX-1]; 1259 struct rtattr *rt = xfrma[XFRMA_SEC_CTX-1];
1260 struct xfrm_policy tmp; 1260 struct xfrm_policy tmp;
@@ -1270,7 +1270,8 @@ static int xfrm_get_policy(struct sk_buff *skb, struct nlmsghdr *nlh,
1270 if ((err = security_xfrm_policy_alloc(&tmp, uctx))) 1270 if ((err = security_xfrm_policy_alloc(&tmp, uctx)))
1271 return err; 1271 return err;
1272 } 1272 }
1273 xp = xfrm_policy_bysel_ctx(type, p->dir, &p->sel, tmp.security, delete); 1273 xp = xfrm_policy_bysel_ctx(type, p->dir, &p->sel, tmp.security,
1274 delete, &err);
1274 security_xfrm_policy_free(&tmp); 1275 security_xfrm_policy_free(&tmp);
1275 } 1276 }
1276 if (xp == NULL) 1277 if (xp == NULL)
@@ -1288,8 +1289,6 @@ static int xfrm_get_policy(struct sk_buff *skb, struct nlmsghdr *nlh,
1288 MSG_DONTWAIT); 1289 MSG_DONTWAIT);
1289 } 1290 }
1290 } else { 1291 } else {
1291 err = security_xfrm_policy_delete(xp);
1292
1293 xfrm_audit_log(NETLINK_CB(skb).loginuid, NETLINK_CB(skb).sid, 1292 xfrm_audit_log(NETLINK_CB(skb).loginuid, NETLINK_CB(skb).sid,
1294 AUDIT_MAC_IPSEC_DELSPD, err ? 0 : 1, xp, NULL); 1293 AUDIT_MAC_IPSEC_DELSPD, err ? 0 : 1, xp, NULL);
1295 1294
@@ -1303,9 +1302,8 @@ static int xfrm_get_policy(struct sk_buff *skb, struct nlmsghdr *nlh,
1303 km_policy_notify(xp, p->dir, &c); 1302 km_policy_notify(xp, p->dir, &c);
1304 } 1303 }
1305 1304
1306 xfrm_pol_put(xp);
1307
1308out: 1305out:
1306 xfrm_pol_put(xp);
1309 return err; 1307 return err;
1310} 1308}
1311 1309
@@ -1502,7 +1500,7 @@ static int xfrm_add_pol_expire(struct sk_buff *skb, struct nlmsghdr *nlh,
1502 return err; 1500 return err;
1503 1501
1504 if (p->index) 1502 if (p->index)
1505 xp = xfrm_policy_byid(type, p->dir, p->index, 0); 1503 xp = xfrm_policy_byid(type, p->dir, p->index, 0, &err);
1506 else { 1504 else {
1507 struct rtattr *rt = xfrma[XFRMA_SEC_CTX-1]; 1505 struct rtattr *rt = xfrma[XFRMA_SEC_CTX-1];
1508 struct xfrm_policy tmp; 1506 struct xfrm_policy tmp;
@@ -1518,13 +1516,14 @@ static int xfrm_add_pol_expire(struct sk_buff *skb, struct nlmsghdr *nlh,
1518 if ((err = security_xfrm_policy_alloc(&tmp, uctx))) 1516 if ((err = security_xfrm_policy_alloc(&tmp, uctx)))
1519 return err; 1517 return err;
1520 } 1518 }
1521 xp = xfrm_policy_bysel_ctx(type, p->dir, &p->sel, tmp.security, 0); 1519 xp = xfrm_policy_bysel_ctx(type, p->dir, &p->sel, tmp.security,
1520 0, &err);
1522 security_xfrm_policy_free(&tmp); 1521 security_xfrm_policy_free(&tmp);
1523 } 1522 }
1524 1523
1525 if (xp == NULL) 1524 if (xp == NULL)
1526 return err; 1525 return -ENOENT;
1527 read_lock(&xp->lock); 1526 read_lock(&xp->lock);
1528 if (xp->dead) { 1527 if (xp->dead) {
1529 read_unlock(&xp->lock); 1528 read_unlock(&xp->lock);
1530 goto out; 1529 goto out;