Commit 236d5929 authored by Jakub Kicinski's avatar Jakub Kicinski
Browse files
Pablo Neira Ayuso says:

====================
Netfilter fixes for net

1) Restore set counter when one of the CPU loses race to add elements
   to sets.

2) After NF_STOLEN, skb might be there no more, update nftables trace
   infra to avoid access to skb in this case. From Florian Westphal.

3) nftables bridge might register a prerouting hook with zero priority,
   br_netfilter incorrectly skips it. Also from Florian.

* git://git.kernel.org/pub/scm/linux/kernel/git/netfilter/nf:
  netfilter: br_netfilter: do not skip all hooks with 0 priority
  netfilter: nf_tables: avoid skb access on nf_stolen
  netfilter: nft_dynset: restore set element counter when failing to update
====================

Link: https://lore.kernel.org/r/


Signed-off-by: default avatarJakub Kicinski <kuba@kernel.org>
parents 9577fc5f c2577862
Loading
Loading
Loading
Loading
+10 −6
Original line number Diff line number Diff line
@@ -1338,24 +1338,28 @@ void nft_unregister_flowtable_type(struct nf_flowtable_type *type);
/**
 *	struct nft_traceinfo - nft tracing information and state
 *
 *	@trace: other struct members are initialised
 *	@nf_trace: copy of skb->nf_trace before rule evaluation
 *	@type: event type (enum nft_trace_types)
 *	@skbid: hash of skb to be used as trace id
 *	@packet_dumped: packet headers sent in a previous traceinfo message
 *	@pkt: pktinfo currently processed
 *	@basechain: base chain currently processed
 *	@chain: chain currently processed
 *	@rule:  rule that was evaluated
 *	@verdict: verdict given by rule
 *	@type: event type (enum nft_trace_types)
 *	@packet_dumped: packet headers sent in a previous traceinfo message
 *	@trace: other struct members are initialised
 */
struct nft_traceinfo {
	bool				trace;
	bool				nf_trace;
	bool				packet_dumped;
	enum nft_trace_types		type:8;
	u32				skbid;
	const struct nft_pktinfo	*pkt;
	const struct nft_base_chain	*basechain;
	const struct nft_chain		*chain;
	const struct nft_rule_dp	*rule;
	const struct nft_verdict	*verdict;
	enum nft_trace_types		type;
	bool				packet_dumped;
	bool				trace;
};

void nft_trace_init(struct nft_traceinfo *info, const struct nft_pktinfo *pkt,
+18 −3
Original line number Diff line number Diff line
@@ -1012,9 +1012,24 @@ int br_nf_hook_thresh(unsigned int hook, struct net *net,
		return okfn(net, sk, skb);

	ops = nf_hook_entries_get_hook_ops(e);
	for (i = 0; i < e->num_hook_entries &&
	      ops[i]->priority <= NF_BR_PRI_BRNF; i++)
		;
	for (i = 0; i < e->num_hook_entries; i++) {
		/* These hooks have already been called */
		if (ops[i]->priority < NF_BR_PRI_BRNF)
			continue;

		/* These hooks have not been called yet, run them. */
		if (ops[i]->priority > NF_BR_PRI_BRNF)
			break;

		/* take a closer look at NF_BR_PRI_BRNF. */
		if (ops[i]->hook == br_nf_pre_routing) {
			/* This hook diverted the skb to this function,
			 * hooks after this have not been run yet.
			 */
			i++;
			break;
		}
	}

	nf_hook_state_init(&state, hook, NFPROTO_BRIDGE, indev, outdev,
			   sk, net, okfn);
+21 −3
Original line number Diff line number Diff line
@@ -25,9 +25,7 @@ static noinline void __nft_trace_packet(struct nft_traceinfo *info,
					const struct nft_chain *chain,
					enum nft_trace_types type)
{
	const struct nft_pktinfo *pkt = info->pkt;

	if (!info->trace || !pkt->skb->nf_trace)
	if (!info->trace || !info->nf_trace)
		return;

	info->chain = chain;
@@ -42,11 +40,24 @@ static inline void nft_trace_packet(struct nft_traceinfo *info,
				    enum nft_trace_types type)
{
	if (static_branch_unlikely(&nft_trace_enabled)) {
		const struct nft_pktinfo *pkt = info->pkt;

		info->nf_trace = pkt->skb->nf_trace;
		info->rule = rule;
		__nft_trace_packet(info, chain, type);
	}
}

static inline void nft_trace_copy_nftrace(struct nft_traceinfo *info)
{
	if (static_branch_unlikely(&nft_trace_enabled)) {
		const struct nft_pktinfo *pkt = info->pkt;

		if (info->trace)
			info->nf_trace = pkt->skb->nf_trace;
	}
}

static void nft_bitwise_fast_eval(const struct nft_expr *expr,
				  struct nft_regs *regs)
{
@@ -85,6 +96,7 @@ static noinline void __nft_trace_verdict(struct nft_traceinfo *info,
					 const struct nft_chain *chain,
					 const struct nft_regs *regs)
{
	const struct nft_pktinfo *pkt = info->pkt;
	enum nft_trace_types type;

	switch (regs->verdict.code) {
@@ -92,8 +104,13 @@ static noinline void __nft_trace_verdict(struct nft_traceinfo *info,
	case NFT_RETURN:
		type = NFT_TRACETYPE_RETURN;
		break;
	case NF_STOLEN:
		type = NFT_TRACETYPE_RULE;
		/* can't access skb->nf_trace; use copy */
		break;
	default:
		type = NFT_TRACETYPE_RULE;
		info->nf_trace = pkt->skb->nf_trace;
		break;
	}

@@ -254,6 +271,7 @@ nft_do_chain(struct nft_pktinfo *pkt, void *priv)
		switch (regs.verdict.code) {
		case NFT_BREAK:
			regs.verdict.code = NFT_CONTINUE;
			nft_trace_copy_nftrace(&info);
			continue;
		case NFT_CONTINUE:
			nft_trace_packet(&info, chain, rule,
+24 −20
Original line number Diff line number Diff line
@@ -7,7 +7,7 @@
#include <linux/module.h>
#include <linux/static_key.h>
#include <linux/hash.h>
#include <linux/jhash.h>
#include <linux/siphash.h>
#include <linux/if_vlan.h>
#include <linux/init.h>
#include <linux/skbuff.h>
@@ -25,22 +25,6 @@
DEFINE_STATIC_KEY_FALSE(nft_trace_enabled);
EXPORT_SYMBOL_GPL(nft_trace_enabled);

static int trace_fill_id(struct sk_buff *nlskb, struct sk_buff *skb)
{
	__be32 id;

	/* using skb address as ID results in a limited number of
	 * values (and quick reuse).
	 *
	 * So we attempt to use as many skb members that will not
	 * change while skb is with netfilter.
	 */
	id = (__be32)jhash_2words(hash32_ptr(skb), skb_get_hash(skb),
				  skb->skb_iif);

	return nla_put_be32(nlskb, NFTA_TRACE_ID, id);
}

static int trace_fill_header(struct sk_buff *nlskb, u16 type,
			     const struct sk_buff *skb,
			     int off, unsigned int len)
@@ -186,6 +170,7 @@ void nft_trace_notify(struct nft_traceinfo *info)
	struct nlmsghdr *nlh;
	struct sk_buff *skb;
	unsigned int size;
	u32 mark = 0;
	u16 event;

	if (!nfnetlink_has_listeners(nft_net(pkt), NFNLGRP_NFTRACE))
@@ -229,7 +214,7 @@ void nft_trace_notify(struct nft_traceinfo *info)
	if (nla_put_be32(skb, NFTA_TRACE_TYPE, htonl(info->type)))
		goto nla_put_failure;

	if (trace_fill_id(skb, pkt->skb))
	if (nla_put_u32(skb, NFTA_TRACE_ID, info->skbid))
		goto nla_put_failure;

	if (nla_put_string(skb, NFTA_TRACE_CHAIN, info->chain->name))
@@ -249,16 +234,24 @@ void nft_trace_notify(struct nft_traceinfo *info)
	case NFT_TRACETYPE_RULE:
		if (nft_verdict_dump(skb, NFTA_TRACE_VERDICT, info->verdict))
			goto nla_put_failure;

		/* pkt->skb undefined iff NF_STOLEN, disable dump */
		if (info->verdict->code == NF_STOLEN)
			info->packet_dumped = true;
		else
			mark = pkt->skb->mark;

		break;
	case NFT_TRACETYPE_POLICY:
		mark = pkt->skb->mark;

		if (nla_put_be32(skb, NFTA_TRACE_POLICY,
				 htonl(info->basechain->policy)))
			goto nla_put_failure;
		break;
	}

	if (pkt->skb->mark &&
	    nla_put_be32(skb, NFTA_TRACE_MARK, htonl(pkt->skb->mark)))
	if (mark && nla_put_be32(skb, NFTA_TRACE_MARK, htonl(mark)))
		goto nla_put_failure;

	if (!info->packet_dumped) {
@@ -283,9 +276,20 @@ void nft_trace_init(struct nft_traceinfo *info, const struct nft_pktinfo *pkt,
		    const struct nft_verdict *verdict,
		    const struct nft_chain *chain)
{
	static siphash_key_t trace_key __read_mostly;
	struct sk_buff *skb = pkt->skb;

	info->basechain = nft_base_chain(chain);
	info->trace = true;
	info->nf_trace = pkt->skb->nf_trace;
	info->packet_dumped = false;
	info->pkt = pkt;
	info->verdict = verdict;

	net_get_random_once(&trace_key, sizeof(trace_key));

	info->skbid = (u32)siphash_3u32(hash32_ptr(skb),
					skb_get_hash(skb),
					skb->skb_iif,
					&trace_key);
}
+2 −0
Original line number Diff line number Diff line
@@ -143,6 +143,7 @@ static bool nft_rhash_update(struct nft_set *set, const u32 *key,
	/* Another cpu may race to insert the element with the same key */
	if (prev) {
		nft_set_elem_destroy(set, he, true);
		atomic_dec(&set->nelems);
		he = prev;
	}

@@ -152,6 +153,7 @@ static bool nft_rhash_update(struct nft_set *set, const u32 *key,

err2:
	nft_set_elem_destroy(set, he, true);
	atomic_dec(&set->nelems);
err1:
	return false;
}