Commit d00551b4 authored by David S. Miller's avatar David S. Miller
Browse files


Steffen Klassert says:

====================
pull request (net): ipsec 2021-08-04

1) Fix a sysbot reported memory leak in xfrm_user_rcv_msg.
   From Pavel Skripkin.

2) Revert "xfrm: policy: Read seqcount outside of rcu-read side
   in xfrm_policy_lookup_bytype". This commit tried to fix a
   lockin bug, but only cured some of the symptoms. A proper
   fix is applied on top of this revert.

3) Fix a locking bug on xfrm state hash resize. A recent change
   on sequence counters accidentally repaced a spinlock by a mutex.
   Fix from Frederic Weisbecker.

4) Fix possible user-memory-access in xfrm_user_rcv_msg_compat().
   From Dmitry Safonov.

5) Add initialiation sefltest fot xfrm_spdattr_type_t.
   From Dmitry Safonov.
====================

Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parents ff0ee9df 480e93e1
Loading
Loading
Loading
Loading
+1 −0
Original line number Diff line number Diff line
@@ -75,6 +75,7 @@ struct netns_xfrm {
#endif
	spinlock_t		xfrm_state_lock;
	seqcount_spinlock_t	xfrm_state_hash_generation;
	seqcount_spinlock_t	xfrm_policy_hash_generation;

	spinlock_t xfrm_policy_lock;
	struct mutex xfrm_cfg_mutex;
+44 −5
Original line number Diff line number Diff line
@@ -298,8 +298,16 @@ static int xfrm_xlate64(struct sk_buff *dst, const struct nlmsghdr *nlh_src)
	len = nlmsg_attrlen(nlh_src, xfrm_msg_min[type]);

	nla_for_each_attr(nla, attrs, len, remaining) {
		int err = xfrm_xlate64_attr(dst, nla);
		int err;

		switch (type) {
		case XFRM_MSG_NEWSPDINFO:
			err = xfrm_nla_cpy(dst, nla, nla_len(nla));
			break;
		default:
			err = xfrm_xlate64_attr(dst, nla);
			break;
		}
		if (err)
			return err;
	}
@@ -341,7 +349,8 @@ static int xfrm_alloc_compat(struct sk_buff *skb, const struct nlmsghdr *nlh_src

/* Calculates len of translated 64-bit message. */
static size_t xfrm_user_rcv_calculate_len64(const struct nlmsghdr *src,
					    struct nlattr *attrs[XFRMA_MAX+1])
					    struct nlattr *attrs[XFRMA_MAX + 1],
					    int maxtype)
{
	size_t len = nlmsg_len(src);

@@ -358,10 +367,20 @@ static size_t xfrm_user_rcv_calculate_len64(const struct nlmsghdr *src,
	case XFRM_MSG_POLEXPIRE:
		len += 8;
		break;
	case XFRM_MSG_NEWSPDINFO:
		/* attirbutes are xfrm_spdattr_type_t, not xfrm_attr_type_t */
		return len;
	default:
		break;
	}

	/* Unexpected for anything, but XFRM_MSG_NEWSPDINFO, please
	 * correct both 64=>32-bit and 32=>64-bit translators to copy
	 * new attributes.
	 */
	if (WARN_ON_ONCE(maxtype))
		return len;

	if (attrs[XFRMA_SA])
		len += 4;
	if (attrs[XFRMA_POLICY])
@@ -440,7 +459,8 @@ static int xfrm_xlate32_attr(void *dst, const struct nlattr *nla,

static int xfrm_xlate32(struct nlmsghdr *dst, const struct nlmsghdr *src,
			struct nlattr *attrs[XFRMA_MAX+1],
			size_t size, u8 type, struct netlink_ext_ack *extack)
			size_t size, u8 type, int maxtype,
			struct netlink_ext_ack *extack)
{
	size_t pos;
	int i;
@@ -520,6 +540,25 @@ static int xfrm_xlate32(struct nlmsghdr *dst, const struct nlmsghdr *src,
	}
	pos = dst->nlmsg_len;

	if (maxtype) {
		/* attirbutes are xfrm_spdattr_type_t, not xfrm_attr_type_t */
		WARN_ON_ONCE(src->nlmsg_type != XFRM_MSG_NEWSPDINFO);

		for (i = 1; i <= maxtype; i++) {
			int err;

			if (!attrs[i])
				continue;

			/* just copy - no need for translation */
			err = xfrm_attr_cpy32(dst, &pos, attrs[i], size,
					nla_len(attrs[i]), nla_len(attrs[i]));
			if (err)
				return err;
		}
		return 0;
	}

	for (i = 1; i < XFRMA_MAX + 1; i++) {
		int err;

@@ -564,7 +603,7 @@ static struct nlmsghdr *xfrm_user_rcv_msg_compat(const struct nlmsghdr *h32,
	if (err < 0)
		return ERR_PTR(err);

	len = xfrm_user_rcv_calculate_len64(h32, attrs);
	len = xfrm_user_rcv_calculate_len64(h32, attrs, maxtype);
	/* The message doesn't need translation */
	if (len == nlmsg_len(h32))
		return NULL;
@@ -574,7 +613,7 @@ static struct nlmsghdr *xfrm_user_rcv_msg_compat(const struct nlmsghdr *h32,
	if (!h64)
		return ERR_PTR(-ENOMEM);

	err = xfrm_xlate32(h64, h32, attrs, len, type, extack);
	err = xfrm_xlate32(h64, h32, attrs, len, type, maxtype, extack);
	if (err < 0) {
		kvfree(h64);
		return ERR_PTR(err);
+1 −1
Original line number Diff line number Diff line
@@ -241,7 +241,7 @@ static void ipcomp_free_tfms(struct crypto_comp * __percpu *tfms)
			break;
	}

	WARN_ON(!pos);
	WARN_ON(list_entry_is_head(pos, &ipcomp_tfms_list, list));

	if (--pos->users)
		return;
+12 −20
Original line number Diff line number Diff line
@@ -155,7 +155,6 @@ static struct xfrm_policy_afinfo const __rcu *xfrm_policy_afinfo[AF_INET6 + 1]
						__read_mostly;

static struct kmem_cache *xfrm_dst_cache __ro_after_init;
static __read_mostly seqcount_mutex_t xfrm_policy_hash_generation;

static struct rhashtable xfrm_policy_inexact_table;
static const struct rhashtable_params xfrm_pol_inexact_params;
@@ -585,7 +584,7 @@ static void xfrm_bydst_resize(struct net *net, int dir)
		return;

	spin_lock_bh(&net->xfrm.xfrm_policy_lock);
	write_seqcount_begin(&xfrm_policy_hash_generation);
	write_seqcount_begin(&net->xfrm.xfrm_policy_hash_generation);

	odst = rcu_dereference_protected(net->xfrm.policy_bydst[dir].table,
				lockdep_is_held(&net->xfrm.xfrm_policy_lock));
@@ -596,7 +595,7 @@ static void xfrm_bydst_resize(struct net *net, int dir)
	rcu_assign_pointer(net->xfrm.policy_bydst[dir].table, ndst);
	net->xfrm.policy_bydst[dir].hmask = nhashmask;

	write_seqcount_end(&xfrm_policy_hash_generation);
	write_seqcount_end(&net->xfrm.xfrm_policy_hash_generation);
	spin_unlock_bh(&net->xfrm.xfrm_policy_lock);

	synchronize_rcu();
@@ -1245,7 +1244,7 @@ static void xfrm_hash_rebuild(struct work_struct *work)
	} while (read_seqretry(&net->xfrm.policy_hthresh.lock, seq));

	spin_lock_bh(&net->xfrm.xfrm_policy_lock);
	write_seqcount_begin(&xfrm_policy_hash_generation);
	write_seqcount_begin(&net->xfrm.xfrm_policy_hash_generation);

	/* make sure that we can insert the indirect policies again before
	 * we start with destructive action.
@@ -1354,7 +1353,7 @@ static void xfrm_hash_rebuild(struct work_struct *work)

out_unlock:
	__xfrm_policy_inexact_flush(net);
	write_seqcount_end(&xfrm_policy_hash_generation);
	write_seqcount_end(&net->xfrm.xfrm_policy_hash_generation);
	spin_unlock_bh(&net->xfrm.xfrm_policy_lock);

	mutex_unlock(&hash_resize_mutex);
@@ -2091,15 +2090,12 @@ static struct xfrm_policy *xfrm_policy_lookup_bytype(struct net *net, u8 type,
	if (unlikely(!daddr || !saddr))
		return NULL;

 retry:
	sequence = read_seqcount_begin(&xfrm_policy_hash_generation);
	rcu_read_lock();

 retry:
	do {
		sequence = read_seqcount_begin(&net->xfrm.xfrm_policy_hash_generation);
		chain = policy_hash_direct(net, daddr, saddr, family, dir);
	if (read_seqcount_retry(&xfrm_policy_hash_generation, sequence)) {
		rcu_read_unlock();
		goto retry;
	}
	} while (read_seqcount_retry(&net->xfrm.xfrm_policy_hash_generation, sequence));

	ret = NULL;
	hlist_for_each_entry_rcu(pol, chain, bydst) {
@@ -2130,15 +2126,11 @@ static struct xfrm_policy *xfrm_policy_lookup_bytype(struct net *net, u8 type,
	}

skip_inexact:
	if (read_seqcount_retry(&xfrm_policy_hash_generation, sequence)) {
		rcu_read_unlock();
	if (read_seqcount_retry(&net->xfrm.xfrm_policy_hash_generation, sequence))
		goto retry;
	}

	if (ret && !xfrm_pol_hold_rcu(ret)) {
		rcu_read_unlock();
	if (ret && !xfrm_pol_hold_rcu(ret))
		goto retry;
	}
fail:
	rcu_read_unlock();

@@ -4089,6 +4081,7 @@ static int __net_init xfrm_net_init(struct net *net)
	/* Initialize the per-net locks here */
	spin_lock_init(&net->xfrm.xfrm_state_lock);
	spin_lock_init(&net->xfrm.xfrm_policy_lock);
	seqcount_spinlock_init(&net->xfrm.xfrm_policy_hash_generation, &net->xfrm.xfrm_policy_lock);
	mutex_init(&net->xfrm.xfrm_cfg_mutex);

	rv = xfrm_statistics_init(net);
@@ -4133,7 +4126,6 @@ void __init xfrm_init(void)
{
	register_pernet_subsys(&xfrm_net_ops);
	xfrm_dev_init();
	seqcount_mutex_init(&xfrm_policy_hash_generation, &hash_resize_mutex);
	xfrm_input_init();

#ifdef CONFIG_XFRM_ESPINTCP
+10 −0
Original line number Diff line number Diff line
@@ -2811,6 +2811,16 @@ static int xfrm_user_rcv_msg(struct sk_buff *skb, struct nlmsghdr *nlh,

	err = link->doit(skb, nlh, attrs);

	/* We need to free skb allocated in xfrm_alloc_compat() before
	 * returning from this function, because consume_skb() won't take
	 * care of frag_list since netlink destructor sets
	 * sbk->head to NULL. (see netlink_skb_destructor())
	 */
	if (skb_has_frag_list(skb)) {
		kfree_skb(skb_shinfo(skb)->frag_list);
		skb_shinfo(skb)->frag_list = NULL;
	}

err:
	kvfree(nlh64);
	return err;
Loading