github-mirror/linux
1
0
Fork 0
mirror of https://github.com/torvalds/linux.git synced 2026-11-06 21:58:01 +01:00
Code Issues Actions Packages Projects Releases Wiki Activity Graph

Merge branch 'xfrm: state: exact mark/mask match for control-plane SA lookups'

Browse Source 
Antony Antony says:

====================
While looking into a XFRM_MSG_MIGRATE_STATE issue reported by Sashiko,
we found the underlying problem generalizes: xfrm allows multiple SAs
to coexist for the same (SPI, daddr, proto) differing only in mark,
and every netlink method that resolves "which SA" - xfrm get_sa(),
del_sa(), update, get_ae, new_ae, expire, migrate - uses the same
wildcard mark match the data path needs. A broader-mask SA can
silently shadow a more specific one:

  # ip xfrm state add ... spi 0x1000 mark 1 mask 1 (SA_target)
  # ip xfrm state add ... spi 0x1000 mark 0 mask 0
    (SA_decoy, catch-all, added after -> bucket head)
  # ip xfrm state delete dst ... proto esp spi 0x1000 mark 1 mask 1
    -> deletes SA_decoy; SA_target survives, untouched

xfrm policy had the same bug, fixed in commit 4f47e8ab6a
("xfrm: policy: match with both mark and mask on user interfaces").

Netlink state lookups using spi use an exact mark/mask match except
for UPDSA; the wildcard match stays for the data path and state_add only.
This series applies that fix across every affected method,
not just XFRM_MSG_MIGRATE_STATE.

Also reject marks with value bits outside the mask (state ADD,
ALLOCSPI, policy ADD, MIGRATE_STATE new mark). These are
misconfigurations anyway, since the extra bits can never match, and
break exact match added here.

This series does not touch PF_KEY, which no longer receives
non-critical fixes.

state_lookup_byaddr is out of scope. This is only fixing spi based
lookups.
====================

Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
This commit is contained in:
Steffen Klassert 2026-10-20 07:41:14 +02:00
commit 835912998c
5 changed files with 165 additions and 50 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

25
Documentation/networking/xfrm/xfrm_migrate_state.rst
View File

@ -27,15 +27,20 @@ SA Identification
=================
The struct is defined in ``include/uapi/linux/xfrm.h``. The SA is looked
up using ``xfrm_state_lookup()`` with ``id.spi``,
``id.daddr``, ``id.proto``, ``id.family``, and
``old_mark.v & old_mark.m`` as the mark key::
up using ``xfrm_state_lookup_exact()`` with ``id.spi``, ``id.daddr``,
``id.proto``, ``id.family``, and an exact match against ``old_mark.v``
and ``old_mark.m``. Unlike the data path, which uses a masked
comparison, this requires the SA's mark and mask to equal ``old_mark``
exactly, so a broad-mask SA is never matched when a more specific one
was intended. If no such SA exists, ``-ESRCH`` is returned.
The layout is::
struct xfrm_user_migrate_state {
struct xfrm_usersa_id id; /* spi, daddr, proto, family */
xfrm_address_t new_daddr;
xfrm_address_t new_saddr;
struct xfrm_mark old_mark; /* SA lookup: key = v & m */
struct xfrm_mark old_mark; /* SA lookup key (exact v/m match) */
struct xfrm_selector new_sel; /* new selector (see Flags) */
__u32 new_reqid;
__u32 flags; /* XFRM_MIGRATE_STATE_* */
@ -72,8 +77,8 @@ inherits the value from the existing SA (omit-to-inherit).
- Description
* - ``XFRMA_MARK``
- Mark on the migrated SA (``struct xfrm_mark``). Absent inherits
``old_mark``. To use no mark on the new SA, send ``XFRMA_MARK``
with ``{0, 0}``.
the mark of the existing SA. To use no mark on the new SA, send
``XFRMA_MARK`` with ``{0, 0}``.
* - ``XFRMA_ENCAP``
- UDP encapsulation template; only ``UDP_ENCAP_ESPINUDP`` is supported.
Set ``encap_type=0`` to remove encap.
@ -259,8 +264,12 @@ Attributes in the notification
Error Handling
==============
If the target SA tuple (new daddr, SPI, proto, new family) is already
occupied, the operation returns ``-EEXIST`` before the migration begins.
If the target SA tuple (new daddr, SPI, proto, new family, mark) is
already occupied, the operation returns ``-EEXIST`` before the migration
begins. "Occupied" includes wildcard shadowing: an existing SA with a
broader mask (e.g. mark 0/0) claims every mark value, so it blocks
migrating to any more specific mark at the same tuple, not just an
exact mark/mask duplicate.
The old SA remains intact and the operation is safe to retry after
resolving the conflict.

7
include/net/xfrm.h
View File

@ -1748,6 +1748,13 @@ struct xfrm_state *xfrm_state_lookup_byaddr(struct net *net, u32 mark,
const xfrm_address_t *saddr,
u8 proto,
unsigned short family);
struct xfrm_state *xfrm_state_lookup_exact(struct net *net, const struct xfrm_mark *mark,
const xfrm_address_t *daddr, __be32 spi,
u8 proto, unsigned short family);
bool xfrm_state_mark_collides(struct net *net, u32 mark,
const xfrm_address_t *daddr, __be32 spi,
u8 proto, unsigned short family,
const struct xfrm_state *self);
#ifdef CONFIG_XFRM_SUB_POLICY
void xfrm_tmpl_sort(struct xfrm_tmpl **dst, struct xfrm_tmpl **src, int n,
unsigned short family);

3
net/xfrm/xfrm_policy.c
View File

@ -1575,9 +1575,6 @@ int xfrm_policy_insert(int dir, struct xfrm_policy *policy, int excl)
struct xfrm_policy *delpol;
struct hlist_head *chain;
/* Sanitize mark before store */
policy->mark.v &= policy->mark.m;
spin_lock_bh(&net->xfrm.xfrm_policy_lock);
chain = policy_hash_bysel(net, &policy->selector, policy->family, dir);
if (chain)

101
net/xfrm/xfrm_state.c
View File

@ -1177,11 +1177,19 @@ static struct xfrm_state *__xfrm_state_lookup_all(const struct xfrm_hash_state_p
return NULL;
}
static struct xfrm_state *__xfrm_state_lookup(const struct xfrm_hash_state_ptrs *state_ptrs,
u32 mark,
const xfrm_address_t *daddr,
__be32 spi, u8 proto,
unsigned short family)
static bool xfrm_state_mark_matches(const struct xfrm_state *x, u32 mark, u32 mask, bool exact)
{
if (exact)
return x->mark.v == mark && x->mark.m == mask;
return (mark & x->mark.m) == x->mark.v;
}
static struct xfrm_state *
__xfrm_state_lookup(const struct xfrm_hash_state_ptrs *state_ptrs,
u32 mark, u32 mask, bool exact,
const xfrm_address_t *daddr,
__be32 spi, u8 proto,
unsigned short family)
{
unsigned int h = __xfrm_spi_hash(daddr, spi, proto, family, state_ptrs->hmask);
struct xfrm_state *x;
@ -1193,7 +1201,7 @@ static struct xfrm_state *__xfrm_state_lookup(const struct xfrm_hash_state_ptrs
!xfrm_addr_equal(&x->id.daddr, daddr, family))
continue;
if ((mark & x->mark.m) != x->mark.v)
if (!xfrm_state_mark_matches(x, mark, mask, exact))
continue;
if (!xfrm_state_hold_rcu(x))
continue;
@ -1203,6 +1211,17 @@ static struct xfrm_state *__xfrm_state_lookup(const struct xfrm_hash_state_ptrs
return NULL;
}
static struct xfrm_state *
__xfrm_state_lookup_exact(const struct xfrm_hash_state_ptrs *state_ptrs,
const struct xfrm_mark *mark,
const xfrm_address_t *daddr,
__be32 spi, u8 proto,
unsigned short family)
{
return __xfrm_state_lookup(state_ptrs, mark->v, mark->m, true,
daddr, spi, proto, family);
}
struct xfrm_state *xfrm_input_state_lookup(struct net *net, u32 mark,
const xfrm_address_t *daddr,
__be32 spi, u8 proto,
@ -1233,7 +1252,7 @@ struct xfrm_state *xfrm_input_state_lookup(struct net *net, u32 mark,
xfrm_hash_ptrs_get(net, &state_ptrs);
x = __xfrm_state_lookup(&state_ptrs, mark, daddr, spi, proto, family);
x = __xfrm_state_lookup(&state_ptrs, mark, 0, false, daddr, spi, proto, family);
if (x) {
spin_lock(&net->xfrm.xfrm_state_lock);
if (x->km.state != XFRM_STATE_VALID) {
@ -1283,7 +1302,7 @@ static struct xfrm_state *__xfrm_state_lookup_byaddr(const struct xfrm_hash_stat
return NULL;
}
static inline struct xfrm_state *
static struct xfrm_state *
__xfrm_state_locate(struct xfrm_state *x, int use_spi, int family)
{
struct xfrm_hash_state_ptrs state_ptrs;
@ -1293,7 +1312,7 @@ __xfrm_state_locate(struct xfrm_state *x, int use_spi, int family)
xfrm_hash_ptrs_get(net, &state_ptrs);
if (use_spi)
return __xfrm_state_lookup(&state_ptrs, mark, &x->id.daddr,
return __xfrm_state_lookup(&state_ptrs, mark, 0, false, &x->id.daddr,
x->id.spi, x->id.proto, family);
else
return __xfrm_state_lookup_byaddr(&state_ptrs, mark,
@ -1732,9 +1751,6 @@ static void __xfrm_state_insert(struct xfrm_state *x)
list_add(&x->km.all, &net->xfrm.state_all);
/* Sanitize mark before store */
x->mark.v &= x->mark.m;
h = xfrm_dst_hash(net, &x->id.daddr, &x->props.saddr,
x->props.reqid, x->props.family);
XFRM_STATE_INSERT(bydst, &x->bydst,
@ -2186,10 +2202,12 @@ int xfrm_state_migrate_install(const struct xfrm_state *x,
struct netlink_ext_ack *extack)
{
if (m->new_family == m->old_family &&
xfrm_addr_equal(&x->id.daddr, &m->new_daddr, m->new_family)) {
xfrm_addr_equal(&x->id.daddr, &m->new_daddr, m->new_family) &&
xc->mark.v == x->mark.v && xc->mark.m == x->mark.m) {
/*
* Care is needed when the destination address of the state is
* to be updated as it is a part of triplet.
* Care is needed when the destination address or mark of the
* state is to be updated, as they are part of the lookup
* triplet.
*/
xfrm_state_insert(xc);
} else {
@ -2383,7 +2401,7 @@ xfrm_state_lookup(struct net *net, u32 mark, const xfrm_address_t *daddr, __be32
rcu_read_lock();
xfrm_hash_ptrs_get(net, &state_ptrs);
x = __xfrm_state_lookup(&state_ptrs, mark, daddr, spi, proto, family);
x = __xfrm_state_lookup(&state_ptrs, mark, 0, false, daddr, spi, proto, family);
rcu_read_unlock();
return x;
}
@ -2407,6 +2425,55 @@ xfrm_state_lookup_byaddr(struct net *net, u32 mark,
}
EXPORT_SYMBOL(xfrm_state_lookup_byaddr);
struct xfrm_state *
xfrm_state_lookup_exact(struct net *net, const struct xfrm_mark *mark,
const xfrm_address_t *daddr, __be32 spi,
u8 proto, unsigned short family)
{
struct xfrm_hash_state_ptrs state_ptrs;
struct xfrm_state *x;
rcu_read_lock();
xfrm_hash_ptrs_get(net, &state_ptrs);
x = __xfrm_state_lookup_exact(&state_ptrs, mark, daddr, spi, proto, family);
rcu_read_unlock();
return x;
}
EXPORT_SYMBOL(xfrm_state_lookup_exact);
/* True if some OTHER state at this tuple would wildcard-match "mark".
* Used by MIGRATE_STATE, which must exclude the state being migrated.
*/
bool xfrm_state_mark_collides(struct net *net, u32 mark,
const xfrm_address_t *daddr, __be32 spi,
u8 proto, unsigned short family,
const struct xfrm_state *self)
{
struct xfrm_hash_state_ptrs state_ptrs;
unsigned int h;
struct xfrm_state *x;
bool collides = false;
rcu_read_lock();
xfrm_hash_ptrs_get(net, &state_ptrs);
h = __xfrm_spi_hash(daddr, spi, proto, family, state_ptrs.hmask);
hlist_for_each_entry_rcu(x, state_ptrs.byspi + h, byspi) {
if (x != self && x->props.family == family &&
x->id.spi == spi && x->id.proto == proto &&
xfrm_addr_equal(&x->id.daddr, daddr, family) &&
(mark & x->mark.m) == x->mark.v) {
collides = true;
break;
}
}
rcu_read_unlock();
return collides;
}
EXPORT_SYMBOL(xfrm_state_mark_collides);
struct xfrm_state *
xfrm_find_acq(struct net *net, const struct xfrm_mark *mark, u8 mode, u32 reqid,
u32 if_id, u32 pcpu_num, u8 proto, const xfrm_address_t *daddr,
@ -3317,7 +3384,7 @@ int xfrm_init_state(struct xfrm_state *x, struct netlink_ext_ack *extack)
if (err)
return err;
err = xfrm_init_replay(x, NULL);
err = xfrm_init_replay(x, extack);
if (err)
return err;

79
net/xfrm/xfrm_user.c
View File

@ -314,6 +314,22 @@ static int verify_selector_prefixlen(u16 family,
}
}
static int verify_mark(struct nlattr **attrs, struct netlink_ext_ack *extack)
{
const struct xfrm_mark *m;
if (!attrs[XFRMA_MARK])
return 0;
m = nla_data(attrs[XFRMA_MARK]);
if ((m->v & m->m) != m->v) {
NL_SET_ERR_MSG(extack, "Invalid mark value/mask combination");
return -EINVAL;
}
return 0;
}
static int verify_newsa_info(struct xfrm_usersa_info *p,
struct nlattr **attrs,
struct netlink_ext_ack *extack)
@ -333,6 +349,10 @@ static int verify_newsa_info(struct xfrm_usersa_info *p,
if (err)
goto out;
err = verify_mark(attrs, extack);
if (err)
goto out;
err = -EINVAL;
switch (p->id.proto) {
case IPPROTO_AH:
@ -1089,11 +1109,12 @@ static struct xfrm_state *xfrm_user_state_lookup(struct net *net,
struct xfrm_state *x = NULL;
struct xfrm_mark m;
int err;
u32 mark = xfrm_mark_get(attrs, &m);
xfrm_mark_get(attrs, &m);
if (xfrm_id_proto_match(p->proto, IPSEC_PROTO_ANY)) {
err = -ESRCH;
x = xfrm_state_lookup(net, mark, &p->daddr, p->spi, p->proto, p->family);
x = xfrm_state_lookup_exact(net, &m, &p->daddr, p->spi, p->proto, p->family);
} else {
xfrm_address_t *saddr = NULL;
@ -1104,7 +1125,7 @@ static struct xfrm_state *xfrm_user_state_lookup(struct net *net,
}
err = -ESRCH;
x = xfrm_state_lookup_byaddr(net, mark,
x = xfrm_state_lookup_byaddr(net, m.v & m.m,
&p->daddr, saddr,
p->proto, p->family);
}
@ -1896,6 +1917,9 @@ static int xfrm_alloc_userspi(struct sk_buff *skb, struct nlmsghdr *nlh,
x = NULL;
err = verify_mark(attrs, extack);
if (err)
goto out_noput;
mark = xfrm_mark_get(attrs, &m);
if (attrs[XFRMA_IF_ID])
@ -2284,6 +2308,9 @@ static int xfrm_add_policy(struct sk_buff *skb, struct nlmsghdr *nlh,
if (err)
return err;
err = verify_sec_ctx_len(attrs, extack);
if (err)
return err;
err = verify_mark(attrs, extack);
if (err)
return err;
@ -2795,14 +2822,13 @@ static int xfrm_get_ae(struct sk_buff *skb, struct nlmsghdr *nlh,
struct sk_buff *r_skb;
int err;
struct km_event c;
u32 mark;
struct xfrm_mark m;
struct xfrm_aevent_id *p = nlmsg_data(nlh);
struct xfrm_usersa_id *id = &p->sa_id;
mark = xfrm_mark_get(attrs, &m);
xfrm_mark_get(attrs, &m);
x = xfrm_state_lookup(net, mark, &id->daddr, id->spi, id->proto, id->family);
x = xfrm_state_lookup_exact(net, &m, &id->daddr, id->spi, id->proto, id->family);
if (x == NULL)
return -ESRCH;
@ -2843,7 +2869,6 @@ static int xfrm_new_ae(struct sk_buff *skb, struct nlmsghdr *nlh,
struct xfrm_state *x;
struct km_event c;
int err = -EINVAL;
u32 mark = 0;
struct xfrm_mark m;
struct xfrm_aevent_id *p = nlmsg_data(nlh);
struct nlattr *rp = attrs[XFRMA_REPLAY_VAL];
@ -2863,9 +2888,10 @@ static int xfrm_new_ae(struct sk_buff *skb, struct nlmsghdr *nlh,
return err;
}
mark = xfrm_mark_get(attrs, &m);
xfrm_mark_get(attrs, &m);
x = xfrm_state_lookup(net, mark, &p->sa_id.daddr, p->sa_id.spi, p->sa_id.proto, p->sa_id.family);
x = xfrm_state_lookup_exact(net, &m, &p->sa_id.daddr, p->sa_id.spi,
p->sa_id.proto, p->sa_id.family);
if (x == NULL)
return -ESRCH;
@ -2999,9 +3025,10 @@ static int xfrm_add_sa_expire(struct sk_buff *skb, struct nlmsghdr *nlh,
struct xfrm_user_expire *ue = nlmsg_data(nlh);
struct xfrm_usersa_info *p = &ue->state;
struct xfrm_mark m;
u32 mark = xfrm_mark_get(attrs, &m);
x = xfrm_state_lookup(net, mark, &p->id.daddr, p->id.spi, p->id.proto, p->family);
xfrm_mark_get(attrs, &m);
x = xfrm_state_lookup_exact(net, &m, &p->id.daddr, p->id.spi, p->id.proto, p->family);
err = -ENOENT;
if (x == NULL)
@ -3370,11 +3397,15 @@ static int xfrm_do_migrate_state(struct sk_buff *skb, struct nlmsghdr *nlh,
return err;
}
err = verify_mark(attrs, extack);
if (err)
return err;
copy_from_user_migrate_state(&m, um);
x = xfrm_state_lookup(net, m.old_mark.v & m.old_mark.m,
&um->id.daddr, um->id.spi,
um->id.proto, um->id.family);
x = xfrm_state_lookup_exact(net, &m.old_mark,
&um->id.daddr, um->id.spi,
um->id.proto, um->id.family);
if (!x) {
NL_SET_ERR_MSG(extack, "Can not find state");
return -ESRCH;
@ -3445,15 +3476,14 @@ static int xfrm_do_migrate_state(struct sk_buff *skb, struct nlmsghdr *nlh,
x->nat_keepalive_interval);
if (m.new_family != um->id.family ||
!xfrm_addr_equal(&m.new_daddr, &um->id.daddr, um->id.family)) {
u32 new_mark_key = m.new_mark ? m.new_mark->v & m.new_mark->m :
m.old_mark.v & m.old_mark.m;
struct xfrm_state *x_new;
!xfrm_addr_equal(&m.new_daddr, &um->id.daddr, um->id.family) ||
(m.new_mark && (m.new_mark->v != x->mark.v ||
m.new_mark->m != x->mark.m))) {
const struct xfrm_mark *new_mark = m.new_mark ? m.new_mark : &x->mark;
x_new = xfrm_state_lookup(net, new_mark_key, &m.new_daddr,
um->id.spi, um->id.proto, m.new_family);
if (x_new) {
xfrm_state_put(x_new);
if (xfrm_state_mark_collides(net, new_mark->v & new_mark->m,
&m.new_daddr, um->id.spi,
um->id.proto, m.new_family, x)) {
NL_SET_ERR_MSG(extack, "New SA tuple already occupied");
err = -EEXIST;
goto out;
@ -3478,6 +3508,7 @@ static int xfrm_do_migrate_state(struct sk_buff *skb, struct nlmsghdr *nlh,
__xfrm_state_delete(x);
spin_unlock_bh(&x->lock);
xfrm_state_hold(xc);
err = xfrm_state_migrate_install(x, xc, &m, extack);
if (err < 0) {
/*
@ -3485,6 +3516,7 @@ static int xfrm_do_migrate_state(struct sk_buff *skb, struct nlmsghdr *nlh,
* free under xfrm_cfg_mutex. Both SAs are gone if it does;
* restoring x would risk SN/IV reuse.
*/
xfrm_state_put(xc);
goto out;
}
@ -3503,10 +3535,13 @@ static int xfrm_do_migrate_state(struct sk_buff *skb, struct nlmsghdr *nlh,
err = 0;
}
xfrm_state_put(xc);
out:
xfrm_state_put(x);
return err;
out_xc:
if (m.xuo)
xfrm_dev_state_delete(xc);
xc->km.state = XFRM_STATE_DEAD;
xfrm_state_put(xc);
xfrm_state_put(x);