Skip to content

Commit 4f47e8a

Browse files
lxinklassert
lxin
authored and
klassert
committed
xfrm: policy: match with both mark and mask on user interfaces
In commit ed17b8d ("xfrm: fix a warning in xfrm_policy_insert_list"), it would take 'priority' to make a policy unique, and allow duplicated policies with different 'priority' to be added, which is not expected by userland, as Tobias reported in strongswan. To fix this duplicated policies issue, and also fix the issue in commit ed17b8d ("xfrm: fix a warning in xfrm_policy_insert_list"), when doing add/del/get/update on user interfaces, this patch is to change to look up a policy with both mark and mask by doing: mark.v == pol->mark.v && mark.m == pol->mark.m and leave the check: (mark & pol->mark.m) == pol->mark.v for tx/rx path only. As the userland expects an exact mark and mask match to manage policies. v1->v2: - make xfrm_policy_mark_match inline and fix the changelog as Tobias suggested. Fixes: 295fae5 ("xfrm: Allow user space manipulation of SPD mark") Fixes: ed17b8d ("xfrm: fix a warning in xfrm_policy_insert_list") Reported-by: Tobias Brunner <[email protected]> Tested-by: Tobias Brunner <[email protected]> Signed-off-by: Xin Long <[email protected]> Signed-off-by: Steffen Klassert <[email protected]>
1 parent 0275875 commit 4f47e8a

File tree

4 files changed

+36
-36
lines changed

4 files changed

+36
-36
lines changed

‎include/net/xfrm.h

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1630,13 +1630,16 @@ int xfrm_policy_walk(struct net *net, struct xfrm_policy_walk *walk,
16301630
void *);
16311631
void xfrm_policy_walk_done(struct xfrm_policy_walk *walk, struct net *net);
16321632
int xfrm_policy_insert(int dir, struct xfrm_policy *policy, int excl);
1633-
struct xfrm_policy *xfrm_policy_bysel_ctx(struct net *net, u32 mark, u32 if_id,
1634-
u8 type, int dir,
1633+
struct xfrm_policy *xfrm_policy_bysel_ctx(struct net *net,
1634+
const struct xfrm_mark *mark,
1635+
u32 if_id, u8 type, int dir,
16351636
struct xfrm_selector *sel,
16361637
struct xfrm_sec_ctx *ctx, int delete,
16371638
int *err);
1638-
struct xfrm_policy *xfrm_policy_byid(struct net *net, u32 mark, u32 if_id, u8,
1639-
int dir, u32 id, int delete, int *err);
1639+
struct xfrm_policy *xfrm_policy_byid(struct net *net,
1640+
const struct xfrm_mark *mark, u32 if_id,
1641+
u8 type, int dir, u32 id, int delete,
1642+
int *err);
16401643
int xfrm_policy_flush(struct net *net, u8 type, bool task_valid);
16411644
void xfrm_policy_hash_rebuild(struct net *net);
16421645
u32 xfrm_get_acqseq(void);

‎net/key/af_key.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2400,7 +2400,7 @@ static int pfkey_spddelete(struct sock *sk, struct sk_buff *skb, const struct sa
24002400
return err;
24012401
}
24022402

2403-
xp = xfrm_policy_bysel_ctx(net, DUMMY_MARK, 0, XFRM_POLICY_TYPE_MAIN,
2403+
xp = xfrm_policy_bysel_ctx(net, &dummy_mark, 0, XFRM_POLICY_TYPE_MAIN,
24042404
pol->sadb_x_policy_dir - 1, &sel, pol_ctx,
24052405
1, &err);
24062406
security_xfrm_policy_free(pol_ctx);
@@ -2651,7 +2651,7 @@ static int pfkey_spdget(struct sock *sk, struct sk_buff *skb, const struct sadb_
26512651
return -EINVAL;
26522652

26532653
delete = (hdr->sadb_msg_type == SADB_X_SPDDELETE2);
2654-
xp = xfrm_policy_byid(net, DUMMY_MARK, 0, XFRM_POLICY_TYPE_MAIN,
2654+
xp = xfrm_policy_byid(net, &dummy_mark, 0, XFRM_POLICY_TYPE_MAIN,
26552655
dir, pol->sadb_x_policy_id, delete, &err);
26562656
if (xp == NULL)
26572657
return -ENOENT;

‎net/xfrm/xfrm_policy.c

Lines changed: 16 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1433,14 +1433,10 @@ static void xfrm_policy_requeue(struct xfrm_policy *old,
14331433
spin_unlock_bh(&pq->hold_queue.lock);
14341434
}
14351435

1436-
static bool xfrm_policy_mark_match(struct xfrm_policy *policy,
1437-
struct xfrm_policy *pol)
1436+
static inline bool xfrm_policy_mark_match(const struct xfrm_mark *mark,
1437+
struct xfrm_policy *pol)
14381438
{
1439-
if (policy->mark.v == pol->mark.v &&
1440-
policy->priority == pol->priority)
1441-
return true;
1442-
1443-
return false;
1439+
return mark->v == pol->mark.v && mark->m == pol->mark.m;
14441440
}
14451441

14461442
static u32 xfrm_pol_bin_key(const void *data, u32 len, u32 seed)
@@ -1503,7 +1499,7 @@ static void xfrm_policy_insert_inexact_list(struct hlist_head *chain,
15031499
if (pol->type == policy->type &&
15041500
pol->if_id == policy->if_id &&
15051501
!selector_cmp(&pol->selector, &policy->selector) &&
1506-
xfrm_policy_mark_match(policy, pol) &&
1502+
xfrm_policy_mark_match(&policy->mark, pol) &&
15071503
xfrm_sec_ctx_match(pol->security, policy->security) &&
15081504
!WARN_ON(delpol)) {
15091505
delpol = pol;
@@ -1538,7 +1534,7 @@ static struct xfrm_policy *xfrm_policy_insert_list(struct hlist_head *chain,
15381534
if (pol->type == policy->type &&
15391535
pol->if_id == policy->if_id &&
15401536
!selector_cmp(&pol->selector, &policy->selector) &&
1541-
xfrm_policy_mark_match(policy, pol) &&
1537+
xfrm_policy_mark_match(&policy->mark, pol) &&
15421538
xfrm_sec_ctx_match(pol->security, policy->security) &&
15431539
!WARN_ON(delpol)) {
15441540
if (excl)
@@ -1610,9 +1606,8 @@ int xfrm_policy_insert(int dir, struct xfrm_policy *policy, int excl)
16101606
EXPORT_SYMBOL(xfrm_policy_insert);
16111607

16121608
static struct xfrm_policy *
1613-
__xfrm_policy_bysel_ctx(struct hlist_head *chain, u32 mark, u32 if_id,
1614-
u8 type, int dir,
1615-
struct xfrm_selector *sel,
1609+
__xfrm_policy_bysel_ctx(struct hlist_head *chain, const struct xfrm_mark *mark,
1610+
u32 if_id, u8 type, int dir, struct xfrm_selector *sel,
16161611
struct xfrm_sec_ctx *ctx)
16171612
{
16181613
struct xfrm_policy *pol;
@@ -1623,7 +1618,7 @@ __xfrm_policy_bysel_ctx(struct hlist_head *chain, u32 mark, u32 if_id,
16231618
hlist_for_each_entry(pol, chain, bydst) {
16241619
if (pol->type == type &&
16251620
pol->if_id == if_id &&
1626-
(mark & pol->mark.m) == pol->mark.v &&
1621+
xfrm_policy_mark_match(mark, pol) &&
16271622
!selector_cmp(sel, &pol->selector) &&
16281623
xfrm_sec_ctx_match(ctx, pol->security))
16291624
return pol;
@@ -1632,11 +1627,10 @@ __xfrm_policy_bysel_ctx(struct hlist_head *chain, u32 mark, u32 if_id,
16321627
return NULL;
16331628
}
16341629

1635-
struct xfrm_policy *xfrm_policy_bysel_ctx(struct net *net, u32 mark, u32 if_id,
1636-
u8 type, int dir,
1637-
struct xfrm_selector *sel,
1638-
struct xfrm_sec_ctx *ctx, int delete,
1639-
int *err)
1630+
struct xfrm_policy *
1631+
xfrm_policy_bysel_ctx(struct net *net, const struct xfrm_mark *mark, u32 if_id,
1632+
u8 type, int dir, struct xfrm_selector *sel,
1633+
struct xfrm_sec_ctx *ctx, int delete, int *err)
16401634
{
16411635
struct xfrm_pol_inexact_bin *bin = NULL;
16421636
struct xfrm_policy *pol, *ret = NULL;
@@ -1703,9 +1697,9 @@ struct xfrm_policy *xfrm_policy_bysel_ctx(struct net *net, u32 mark, u32 if_id,
17031697
}
17041698
EXPORT_SYMBOL(xfrm_policy_bysel_ctx);
17051699

1706-
struct xfrm_policy *xfrm_policy_byid(struct net *net, u32 mark, u32 if_id,
1707-
u8 type, int dir, u32 id, int delete,
1708-
int *err)
1700+
struct xfrm_policy *
1701+
xfrm_policy_byid(struct net *net, const struct xfrm_mark *mark, u32 if_id,
1702+
u8 type, int dir, u32 id, int delete, int *err)
17091703
{
17101704
struct xfrm_policy *pol, *ret;
17111705
struct hlist_head *chain;
@@ -1720,8 +1714,7 @@ struct xfrm_policy *xfrm_policy_byid(struct net *net, u32 mark, u32 if_id,
17201714
ret = NULL;
17211715
hlist_for_each_entry(pol, chain, byidx) {
17221716
if (pol->type == type && pol->index == id &&
1723-
pol->if_id == if_id &&
1724-
(mark & pol->mark.m) == pol->mark.v) {
1717+
pol->if_id == if_id && xfrm_policy_mark_match(mark, pol)) {
17251718
xfrm_pol_hold(pol);
17261719
if (delete) {
17271720
*err = security_xfrm_policy_delete(

‎net/xfrm/xfrm_user.c

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1863,7 +1863,6 @@ static int xfrm_get_policy(struct sk_buff *skb, struct nlmsghdr *nlh,
18631863
struct km_event c;
18641864
int delete;
18651865
struct xfrm_mark m;
1866-
u32 mark = xfrm_mark_get(attrs, &m);
18671866
u32 if_id = 0;
18681867

18691868
p = nlmsg_data(nlh);
@@ -1880,8 +1879,11 @@ static int xfrm_get_policy(struct sk_buff *skb, struct nlmsghdr *nlh,
18801879
if (attrs[XFRMA_IF_ID])
18811880
if_id = nla_get_u32(attrs[XFRMA_IF_ID]);
18821881

1882+
xfrm_mark_get(attrs, &m);
1883+
18831884
if (p->index)
1884-
xp = xfrm_policy_byid(net, mark, if_id, type, p->dir, p->index, delete, &err);
1885+
xp = xfrm_policy_byid(net, &m, if_id, type, p->dir,
1886+
p->index, delete, &err);
18851887
else {
18861888
struct nlattr *rt = attrs[XFRMA_SEC_CTX];
18871889
struct xfrm_sec_ctx *ctx;
@@ -1898,8 +1900,8 @@ static int xfrm_get_policy(struct sk_buff *skb, struct nlmsghdr *nlh,
18981900
if (err)
18991901
return err;
19001902
}
1901-
xp = xfrm_policy_bysel_ctx(net, mark, if_id, type, p->dir, &p->sel,
1902-
ctx, delete, &err);
1903+
xp = xfrm_policy_bysel_ctx(net, &m, if_id, type, p->dir,
1904+
&p->sel, ctx, delete, &err);
19031905
security_xfrm_policy_free(ctx);
19041906
}
19051907
if (xp == NULL)
@@ -2166,7 +2168,6 @@ static int xfrm_add_pol_expire(struct sk_buff *skb, struct nlmsghdr *nlh,
21662168
u8 type = XFRM_POLICY_TYPE_MAIN;
21672169
int err = -ENOENT;
21682170
struct xfrm_mark m;
2169-
u32 mark = xfrm_mark_get(attrs, &m);
21702171
u32 if_id = 0;
21712172

21722173
err = copy_from_user_policy_type(&type, attrs);
@@ -2180,8 +2181,11 @@ static int xfrm_add_pol_expire(struct sk_buff *skb, struct nlmsghdr *nlh,
21802181
if (attrs[XFRMA_IF_ID])
21812182
if_id = nla_get_u32(attrs[XFRMA_IF_ID]);
21822183

2184+
xfrm_mark_get(attrs, &m);
2185+
21832186
if (p->index)
2184-
xp = xfrm_policy_byid(net, mark, if_id, type, p->dir, p->index, 0, &err);
2187+
xp = xfrm_policy_byid(net, &m, if_id, type, p->dir, p->index,
2188+
0, &err);
21852189
else {
21862190
struct nlattr *rt = attrs[XFRMA_SEC_CTX];
21872191
struct xfrm_sec_ctx *ctx;
@@ -2198,7 +2202,7 @@ static int xfrm_add_pol_expire(struct sk_buff *skb, struct nlmsghdr *nlh,
21982202
if (err)
21992203
return err;
22002204
}
2201-
xp = xfrm_policy_bysel_ctx(net, mark, if_id, type, p->dir,
2205+
xp = xfrm_policy_bysel_ctx(net, &m, if_id, type, p->dir,
22022206
&p->sel, ctx, 0, &err);
22032207
security_xfrm_policy_free(ctx);
22042208
}

0 commit comments

Comments
 (0)