diff --git a/Documentation/networking/xfrm/xfrm_migrate_state.rst b/Documentation/networking/xfrm/xfrm_migrate_state.rst index 9d53cb22b007..ca2c9e5aaf33 100644 --- a/Documentation/networking/xfrm/xfrm_migrate_state.rst +++ b/Documentation/networking/xfrm/xfrm_migrate_state.rst @@ -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. diff --git a/include/net/xfrm.h b/include/net/xfrm.h index a6d69aaa6cd2..26dd4b570588 100644 --- a/include/net/xfrm.h +++ b/include/net/xfrm.h @@ -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); diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c index f6f40ba713d5..0f6fcea28bcd 100644 --- a/net/xfrm/xfrm_policy.c +++ b/net/xfrm/xfrm_policy.c @@ -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) diff --git a/net/xfrm/xfrm_state.c b/net/xfrm/xfrm_state.c index e45aa1ed5b96..0555e5d796ed 100644 --- a/net/xfrm/xfrm_state.c +++ b/net/xfrm/xfrm_state.c @@ -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; diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c index a2587c7e796b..a5fbe38839c4 100644 --- a/net/xfrm/xfrm_user.c +++ b/net/xfrm/xfrm_user.c @@ -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);