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


netfilter pull request 23-06-08

Pablo Neira Ayuso says:

====================
The following patchset contains Netfilter fixes for net:

1) Add commit and abort set operation to pipapo set abort path.

2) Bail out immediately in case of ENOMEM in nfnetlink batch.

3) Incorrect error path handling when creating a new rule leads to
   dangling pointer in set transaction list.
====================

Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parents b403643d 1240eb93
Loading
Loading
Loading
Loading
+3 −1
Original line number Diff line number Diff line
@@ -462,7 +462,8 @@ struct nft_set_ops {
					       const struct nft_set *set,
					       const struct nft_set_elem *elem,
					       unsigned int flags);

	void				(*commit)(const struct nft_set *set);
	void				(*abort)(const struct nft_set *set);
	u64				(*privsize)(const struct nlattr * const nla[],
						    const struct nft_set_desc *desc);
	bool				(*estimate)(const struct nft_set_desc *desc,
@@ -557,6 +558,7 @@ struct nft_set {
	u16				policy;
	u16				udlen;
	unsigned char			*udata;
	struct list_head		pending_update;
	/* runtime data below here */
	const struct nft_set_ops	*ops ____cacheline_aligned;
	u16				flags:14,
+58 −1
Original line number Diff line number Diff line
@@ -3844,7 +3844,8 @@ static int nf_tables_newrule(struct sk_buff *skb, const struct nfnl_info *info,
	if (flow)
		nft_flow_rule_destroy(flow);
err_release_rule:
	nf_tables_rule_release(&ctx, rule);
	nft_rule_expr_deactivate(&ctx, rule, NFT_TRANS_PREPARE);
	nf_tables_rule_destroy(&ctx, rule);
err_release_expr:
	for (i = 0; i < n; i++) {
		if (expr_info[i].ops) {
@@ -4919,6 +4920,7 @@ static int nf_tables_newset(struct sk_buff *skb, const struct nfnl_info *info,

	set->num_exprs = num_exprs;
	set->handle = nf_tables_alloc_handle(table);
	INIT_LIST_HEAD(&set->pending_update);

	err = nft_trans_set_add(&ctx, NFT_MSG_NEWSET, set);
	if (err < 0)
@@ -9275,10 +9277,25 @@ static void nf_tables_commit_audit_log(struct list_head *adl, u32 generation)
	}
}

static void nft_set_commit_update(struct list_head *set_update_list)
{
	struct nft_set *set, *next;

	list_for_each_entry_safe(set, next, set_update_list, pending_update) {
		list_del_init(&set->pending_update);

		if (!set->ops->commit)
			continue;

		set->ops->commit(set);
	}
}

static int nf_tables_commit(struct net *net, struct sk_buff *skb)
{
	struct nftables_pernet *nft_net = nft_pernet(net);
	struct nft_trans *trans, *next;
	LIST_HEAD(set_update_list);
	struct nft_trans_elem *te;
	struct nft_chain *chain;
	struct nft_table *table;
@@ -9453,6 +9470,11 @@ static int nf_tables_commit(struct net *net, struct sk_buff *skb)
			nf_tables_setelem_notify(&trans->ctx, te->set,
						 &te->elem,
						 NFT_MSG_NEWSETELEM);
			if (te->set->ops->commit &&
			    list_empty(&te->set->pending_update)) {
				list_add_tail(&te->set->pending_update,
					      &set_update_list);
			}
			nft_trans_destroy(trans);
			break;
		case NFT_MSG_DELSETELEM:
@@ -9467,6 +9489,11 @@ static int nf_tables_commit(struct net *net, struct sk_buff *skb)
				atomic_dec(&te->set->nelems);
				te->set->ndeact--;
			}
			if (te->set->ops->commit &&
			    list_empty(&te->set->pending_update)) {
				list_add_tail(&te->set->pending_update,
					      &set_update_list);
			}
			break;
		case NFT_MSG_NEWOBJ:
			if (nft_trans_obj_update(trans)) {
@@ -9529,6 +9556,8 @@ static int nf_tables_commit(struct net *net, struct sk_buff *skb)
		}
	}

	nft_set_commit_update(&set_update_list);

	nft_commit_notify(net, NETLINK_CB(skb).portid);
	nf_tables_gen_notify(net, skb, NFT_MSG_NEWGEN);
	nf_tables_commit_audit_log(&adl, nft_net->base_seq);
@@ -9588,10 +9617,25 @@ static void nf_tables_abort_release(struct nft_trans *trans)
	kfree(trans);
}

static void nft_set_abort_update(struct list_head *set_update_list)
{
	struct nft_set *set, *next;

	list_for_each_entry_safe(set, next, set_update_list, pending_update) {
		list_del_init(&set->pending_update);

		if (!set->ops->abort)
			continue;

		set->ops->abort(set);
	}
}

static int __nf_tables_abort(struct net *net, enum nfnl_abort_action action)
{
	struct nftables_pernet *nft_net = nft_pernet(net);
	struct nft_trans *trans, *next;
	LIST_HEAD(set_update_list);
	struct nft_trans_elem *te;

	if (action == NFNL_ABORT_VALIDATE &&
@@ -9701,6 +9745,12 @@ static int __nf_tables_abort(struct net *net, enum nfnl_abort_action action)
			nft_setelem_remove(net, te->set, &te->elem);
			if (!nft_setelem_is_catchall(te->set, &te->elem))
				atomic_dec(&te->set->nelems);

			if (te->set->ops->abort &&
			    list_empty(&te->set->pending_update)) {
				list_add_tail(&te->set->pending_update,
					      &set_update_list);
			}
			break;
		case NFT_MSG_DELSETELEM:
		case NFT_MSG_DESTROYSETELEM:
@@ -9711,6 +9761,11 @@ static int __nf_tables_abort(struct net *net, enum nfnl_abort_action action)
			if (!nft_setelem_is_catchall(te->set, &te->elem))
				te->set->ndeact--;

			if (te->set->ops->abort &&
			    list_empty(&te->set->pending_update)) {
				list_add_tail(&te->set->pending_update,
					      &set_update_list);
			}
			nft_trans_destroy(trans);
			break;
		case NFT_MSG_NEWOBJ:
@@ -9753,6 +9808,8 @@ static int __nf_tables_abort(struct net *net, enum nfnl_abort_action action)
		}
	}

	nft_set_abort_update(&set_update_list);

	synchronize_rcu();

	list_for_each_entry_safe_reverse(trans, next,
+2 −1
Original line number Diff line number Diff line
@@ -533,7 +533,8 @@ static void nfnetlink_rcv_batch(struct sk_buff *skb, struct nlmsghdr *nlh,
			 * processed, this avoids that the same error is
			 * reported several times when replaying the batch.
			 */
			if (nfnl_err_add(&err_list, nlh, err, &extack) < 0) {
			if (err == -ENOMEM ||
			    nfnl_err_add(&err_list, nlh, err, &extack) < 0) {
				/* We failed to enqueue an error, reset the
				 * list of errors and send OOM to userspace
				 * pointing to the batch header.
+40 −15
Original line number Diff line number Diff line
@@ -1600,17 +1600,10 @@ static void pipapo_free_fields(struct nft_pipapo_match *m)
	}
}

/**
 * pipapo_reclaim_match - RCU callback to free fields from old matching data
 * @rcu:	RCU head
 */
static void pipapo_reclaim_match(struct rcu_head *rcu)
static void pipapo_free_match(struct nft_pipapo_match *m)
{
	struct nft_pipapo_match *m;
	int i;

	m = container_of(rcu, struct nft_pipapo_match, rcu);

	for_each_possible_cpu(i)
		kfree(*per_cpu_ptr(m->scratch, i));

@@ -1625,7 +1618,19 @@ static void pipapo_reclaim_match(struct rcu_head *rcu)
}

/**
 * pipapo_commit() - Replace lookup data with current working copy
 * pipapo_reclaim_match - RCU callback to free fields from old matching data
 * @rcu:	RCU head
 */
static void pipapo_reclaim_match(struct rcu_head *rcu)
{
	struct nft_pipapo_match *m;

	m = container_of(rcu, struct nft_pipapo_match, rcu);
	pipapo_free_match(m);
}

/**
 * nft_pipapo_commit() - Replace lookup data with current working copy
 * @set:	nftables API set representation
 *
 * While at it, check if we should perform garbage collection on the working
@@ -1635,7 +1640,7 @@ static void pipapo_reclaim_match(struct rcu_head *rcu)
 * We also need to create a new working copy for subsequent insertions and
 * deletions.
 */
static void pipapo_commit(const struct nft_set *set)
static void nft_pipapo_commit(const struct nft_set *set)
{
	struct nft_pipapo *priv = nft_set_priv(set);
	struct nft_pipapo_match *new_clone, *old;
@@ -1660,6 +1665,26 @@ static void pipapo_commit(const struct nft_set *set)
	priv->clone = new_clone;
}

static void nft_pipapo_abort(const struct nft_set *set)
{
	struct nft_pipapo *priv = nft_set_priv(set);
	struct nft_pipapo_match *new_clone, *m;

	if (!priv->dirty)
		return;

	m = rcu_dereference(priv->match);

	new_clone = pipapo_clone(m);
	if (IS_ERR(new_clone))
		return;

	priv->dirty = false;

	pipapo_free_match(priv->clone);
	priv->clone = new_clone;
}

/**
 * nft_pipapo_activate() - Mark element reference as active given key, commit
 * @net:	Network namespace
@@ -1667,8 +1692,7 @@ static void pipapo_commit(const struct nft_set *set)
 * @elem:	nftables API element representation containing key data
 *
 * On insertion, elements are added to a copy of the matching data currently
 * in use for lookups, and not directly inserted into current lookup data, so
 * we'll take care of that by calling pipapo_commit() here. Both
 * in use for lookups, and not directly inserted into current lookup data. Both
 * nft_pipapo_insert() and nft_pipapo_activate() are called once for each
 * element, hence we can't purpose either one as a real commit operation.
 */
@@ -1684,8 +1708,6 @@ static void nft_pipapo_activate(const struct net *net,

	nft_set_elem_change_active(net, set, &e->ext);
	nft_set_elem_clear_busy(&e->ext);

	pipapo_commit(set);
}

/**
@@ -1931,7 +1953,6 @@ static void nft_pipapo_remove(const struct net *net, const struct nft_set *set,
		if (i == m->field_count) {
			priv->dirty = true;
			pipapo_drop(m, rulemap);
			pipapo_commit(set);
			return;
		}

@@ -2230,6 +2251,8 @@ const struct nft_set_type nft_set_pipapo_type = {
		.init		= nft_pipapo_init,
		.destroy	= nft_pipapo_destroy,
		.gc_init	= nft_pipapo_gc_init,
		.commit		= nft_pipapo_commit,
		.abort		= nft_pipapo_abort,
		.elemsize	= offsetof(struct nft_pipapo_elem, ext),
	},
};
@@ -2252,6 +2275,8 @@ const struct nft_set_type nft_set_pipapo_avx2_type = {
		.init		= nft_pipapo_init,
		.destroy	= nft_pipapo_destroy,
		.gc_init	= nft_pipapo_gc_init,
		.commit		= nft_pipapo_commit,
		.abort		= nft_pipapo_abort,
		.elemsize	= offsetof(struct nft_pipapo_elem, ext),
	},
};