Skip to content

Commit 0b91fda

Browse files
pchaignoklassert
authored andcommitted
xfrm: Sanitize marks before insert
Prior to this patch, the mark is sanitized (applying the state's mask to the state's value) only on inserts when checking if a conflicting XFRM state or policy exists. We discovered in Cilium that this same sanitization does not occur in the hot-path __xfrm_state_lookup. In the hot-path, the sk_buff's mark is simply compared to the state's value: if ((mark & x->mark.m) != x->mark.v) continue; Therefore, users can define unsanitized marks (ex. 0xf42/0xf00) which will never match any packet. This commit updates __xfrm_state_insert and xfrm_policy_insert to store the sanitized marks, thus removing this footgun. This has the side effect of changing the ip output, as the returned mark will have the mask applied to it when printed. Fixes: 3d6acfa ("xfrm: SA lookups with mark") Signed-off-by: Paul Chaignon <[email protected]> Signed-off-by: Louis DeLosSantos <[email protected]> Co-developed-by: Louis DeLosSantos <[email protected]> Signed-off-by: Steffen Klassert <[email protected]>
1 parent 417fae2 commit 0b91fda

File tree

2 files changed

+6
-0
lines changed

2 files changed

+6
-0
lines changed

net/xfrm/xfrm_policy.c

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1581,6 +1581,9 @@ int xfrm_policy_insert(int dir, struct xfrm_policy *policy, int excl)
15811581
struct xfrm_policy *delpol;
15821582
struct hlist_head *chain;
15831583

1584+
/* Sanitize mark before store */
1585+
policy->mark.v &= policy->mark.m;
1586+
15841587
spin_lock_bh(&net->xfrm.xfrm_policy_lock);
15851588
chain = policy_hash_bysel(net, &policy->selector, policy->family, dir);
15861589
if (chain)

net/xfrm/xfrm_state.c

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1718,6 +1718,9 @@ static void __xfrm_state_insert(struct xfrm_state *x)
17181718

17191719
list_add(&x->km.all, &net->xfrm.state_all);
17201720

1721+
/* Sanitize mark before store */
1722+
x->mark.v &= x->mark.m;
1723+
17211724
h = xfrm_dst_hash(net, &x->id.daddr, &x->props.saddr,
17221725
x->props.reqid, x->props.family);
17231726
XFRM_STATE_INSERT(bydst, &x->bydst, net->xfrm.state_bydst + h,

0 commit comments

Comments
 (0)