Browse Source

Merge git://git.kernel.org/pub/scm/linux/kernel/git/pablo/nf

Pablo Neira Ayuso says:

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

The following patchset contains Netfilter fixes for your net tree,
they are:

1) Don't pick fixed hash implementation for NFT_SET_EVAL sets, otherwise
   userspace hits EOPNOTSUPP with valid rules using the meter statement,
   from Florian Westphal.

2) If you send a batch that flushes the existing ruleset (that contains
   a NAT chain) and the new ruleset definition comes with a new NAT
   chain, don't bogusly hit EBUSY. Also from Florian.

3) Missing netlink policy attribute validation, from Florian.

4) Detach conntrack template from skbuff if IP_NODEFRAG is set on,
   from Paolo Abeni.

5) Cache device names in flowtable object, otherwise we may end up
   walking over devices going aways given no rtnl_lock is held.

6) Fix incorrect net_device ingress with ingress hooks.

7) Fix crash when trying to read more data than available in UDP
   packets from the nf_socket infrastructure, from Subash.
====================

Signed-off-by: David S. Miller <davem@davemloft.net>
David S. Miller 7 years ago
parent
commit
b9ee96b45f

+ 4 - 0
include/net/netfilter/nf_tables.h

@@ -1068,6 +1068,8 @@ struct nft_object_ops {
 int nft_register_obj(struct nft_object_type *obj_type);
 int nft_register_obj(struct nft_object_type *obj_type);
 void nft_unregister_obj(struct nft_object_type *obj_type);
 void nft_unregister_obj(struct nft_object_type *obj_type);
 
 
+#define NFT_FLOWTABLE_DEVICE_MAX	8
+
 /**
 /**
  *	struct nft_flowtable - nf_tables flow table
  *	struct nft_flowtable - nf_tables flow table
  *
  *
@@ -1080,6 +1082,7 @@ void nft_unregister_obj(struct nft_object_type *obj_type);
  *	@genmask: generation mask
  *	@genmask: generation mask
  *	@use: number of references to this flow table
  *	@use: number of references to this flow table
  * 	@handle: unique object handle
  * 	@handle: unique object handle
+ *	@dev_name: array of device names
  *	@data: rhashtable and garbage collector
  *	@data: rhashtable and garbage collector
  * 	@ops: array of hooks
  * 	@ops: array of hooks
  */
  */
@@ -1093,6 +1096,7 @@ struct nft_flowtable {
 	u32				genmask:2,
 	u32				genmask:2,
 					use:30;
 					use:30;
 	u64				handle;
 	u64				handle;
+	char				*dev_name[NFT_FLOWTABLE_DEVICE_MAX];
 	/* runtime data below here */
 	/* runtime data below here */
 	struct nf_hook_ops		*ops ____cacheline_aligned;
 	struct nf_hook_ops		*ops ____cacheline_aligned;
 	struct nf_flowtable		data;
 	struct nf_flowtable		data;

+ 13 - 1
net/ipv4/netfilter/nf_conntrack_l3proto_ipv4.c

@@ -154,8 +154,20 @@ static unsigned int ipv4_conntrack_local(void *priv,
 					 struct sk_buff *skb,
 					 struct sk_buff *skb,
 					 const struct nf_hook_state *state)
 					 const struct nf_hook_state *state)
 {
 {
-	if (ip_is_fragment(ip_hdr(skb))) /* IP_NODEFRAG setsockopt set */
+	if (ip_is_fragment(ip_hdr(skb))) { /* IP_NODEFRAG setsockopt set */
+		enum ip_conntrack_info ctinfo;
+		struct nf_conn *tmpl;
+
+		tmpl = nf_ct_get(skb, &ctinfo);
+		if (tmpl && nf_ct_is_template(tmpl)) {
+			/* when skipping ct, clear templates to avoid fooling
+			 * later targets/matches
+			 */
+			skb->_nfct = 0;
+			nf_ct_put(tmpl);
+		}
 		return NF_ACCEPT;
 		return NF_ACCEPT;
+	}
 
 
 	return nf_conntrack_in(state->net, PF_INET, state->hook, skb);
 	return nf_conntrack_in(state->net, PF_INET, state->hook, skb);
 }
 }

+ 4 - 2
net/ipv4/netfilter/nf_socket_ipv4.c

@@ -108,10 +108,12 @@ struct sock *nf_sk_lookup_slow_v4(struct net *net, const struct sk_buff *skb,
 	int doff = 0;
 	int doff = 0;
 
 
 	if (iph->protocol == IPPROTO_UDP || iph->protocol == IPPROTO_TCP) {
 	if (iph->protocol == IPPROTO_UDP || iph->protocol == IPPROTO_TCP) {
-		struct udphdr _hdr, *hp;
+		struct tcphdr _hdr;
+		struct udphdr *hp;
 
 
 		hp = skb_header_pointer(skb, ip_hdrlen(skb),
 		hp = skb_header_pointer(skb, ip_hdrlen(skb),
-					sizeof(_hdr), &_hdr);
+					iph->protocol == IPPROTO_UDP ?
+					sizeof(*hp) : sizeof(_hdr), &_hdr);
 		if (hp == NULL)
 		if (hp == NULL)
 			return NULL;
 			return NULL;
 
 

+ 4 - 2
net/ipv6/netfilter/nf_socket_ipv6.c

@@ -116,9 +116,11 @@ struct sock *nf_sk_lookup_slow_v6(struct net *net, const struct sk_buff *skb,
 	}
 	}
 
 
 	if (tproto == IPPROTO_UDP || tproto == IPPROTO_TCP) {
 	if (tproto == IPPROTO_UDP || tproto == IPPROTO_TCP) {
-		struct udphdr _hdr, *hp;
+		struct tcphdr _hdr;
+		struct udphdr *hp;
 
 
-		hp = skb_header_pointer(skb, thoff, sizeof(_hdr), &_hdr);
+		hp = skb_header_pointer(skb, thoff, tproto == IPPROTO_UDP ?
+					sizeof(*hp) : sizeof(_hdr), &_hdr);
 		if (hp == NULL)
 		if (hp == NULL)
 			return NULL;
 			return NULL;
 
 

+ 83 - 23
net/netfilter/nf_tables_api.c

@@ -74,15 +74,77 @@ static void nft_trans_destroy(struct nft_trans *trans)
 	kfree(trans);
 	kfree(trans);
 }
 }
 
 
+/* removal requests are queued in the commit_list, but not acted upon
+ * until after all new rules are in place.
+ *
+ * Therefore, nf_register_net_hook(net, &nat_hook) runs before pending
+ * nf_unregister_net_hook().
+ *
+ * nf_register_net_hook thus fails if a nat hook is already in place
+ * even if the conflicting hook is about to be removed.
+ *
+ * If collision is detected, search commit_log for DELCHAIN matching
+ * the new nat hooknum; if we find one collision is temporary:
+ *
+ * Either transaction is aborted (new/colliding hook is removed), or
+ * transaction is committed (old hook is removed).
+ */
+static bool nf_tables_allow_nat_conflict(const struct net *net,
+					 const struct nf_hook_ops *ops)
+{
+	const struct nft_trans *trans;
+	bool ret = false;
+
+	if (!ops->nat_hook)
+		return false;
+
+	list_for_each_entry(trans, &net->nft.commit_list, list) {
+		const struct nf_hook_ops *pending_ops;
+		const struct nft_chain *pending;
+
+		if (trans->msg_type != NFT_MSG_NEWCHAIN &&
+		    trans->msg_type != NFT_MSG_DELCHAIN)
+			continue;
+
+		pending = trans->ctx.chain;
+		if (!nft_is_base_chain(pending))
+			continue;
+
+		pending_ops = &nft_base_chain(pending)->ops;
+		if (pending_ops->nat_hook &&
+		    pending_ops->pf == ops->pf &&
+		    pending_ops->hooknum == ops->hooknum) {
+			/* other hook registration already pending? */
+			if (trans->msg_type == NFT_MSG_NEWCHAIN)
+				return false;
+
+			ret = true;
+		}
+	}
+
+	return ret;
+}
+
 static int nf_tables_register_hook(struct net *net,
 static int nf_tables_register_hook(struct net *net,
 				   const struct nft_table *table,
 				   const struct nft_table *table,
 				   struct nft_chain *chain)
 				   struct nft_chain *chain)
 {
 {
+	struct nf_hook_ops *ops;
+	int ret;
+
 	if (table->flags & NFT_TABLE_F_DORMANT ||
 	if (table->flags & NFT_TABLE_F_DORMANT ||
 	    !nft_is_base_chain(chain))
 	    !nft_is_base_chain(chain))
 		return 0;
 		return 0;
 
 
-	return nf_register_net_hook(net, &nft_base_chain(chain)->ops);
+	ops = &nft_base_chain(chain)->ops;
+	ret = nf_register_net_hook(net, ops);
+	if (ret == -EBUSY && nf_tables_allow_nat_conflict(net, ops)) {
+		ops->nat_hook = false;
+		ret = nf_register_net_hook(net, ops);
+		ops->nat_hook = true;
+	}
+
+	return ret;
 }
 }
 
 
 static void nf_tables_unregister_hook(struct net *net,
 static void nf_tables_unregister_hook(struct net *net,
@@ -1226,8 +1288,6 @@ static void nf_tables_chain_destroy(struct nft_chain *chain)
 		free_percpu(basechain->stats);
 		free_percpu(basechain->stats);
 		if (basechain->stats)
 		if (basechain->stats)
 			static_branch_dec(&nft_counters_enabled);
 			static_branch_dec(&nft_counters_enabled);
-		if (basechain->ops.dev != NULL)
-			dev_put(basechain->ops.dev);
 		kfree(chain->name);
 		kfree(chain->name);
 		kfree(basechain);
 		kfree(basechain);
 	} else {
 	} else {
@@ -1294,7 +1354,7 @@ static int nft_chain_parse_hook(struct net *net,
 		}
 		}
 
 
 		nla_strlcpy(ifname, ha[NFTA_HOOK_DEV], IFNAMSIZ);
 		nla_strlcpy(ifname, ha[NFTA_HOOK_DEV], IFNAMSIZ);
-		dev = dev_get_by_name(net, ifname);
+		dev = __dev_get_by_name(net, ifname);
 		if (!dev) {
 		if (!dev) {
 			module_put(type->owner);
 			module_put(type->owner);
 			return -ENOENT;
 			return -ENOENT;
@@ -1311,8 +1371,6 @@ static int nft_chain_parse_hook(struct net *net,
 static void nft_chain_release_hook(struct nft_chain_hook *hook)
 static void nft_chain_release_hook(struct nft_chain_hook *hook)
 {
 {
 	module_put(hook->type->owner);
 	module_put(hook->type->owner);
-	if (hook->dev != NULL)
-		dev_put(hook->dev);
 }
 }
 
 
 static int nf_tables_addchain(struct nft_ctx *ctx, u8 family, u8 genmask,
 static int nf_tables_addchain(struct nft_ctx *ctx, u8 family, u8 genmask,
@@ -1911,6 +1969,7 @@ static const struct nla_policy nft_rule_policy[NFTA_RULE_MAX + 1] = {
 	[NFTA_RULE_POSITION]	= { .type = NLA_U64 },
 	[NFTA_RULE_POSITION]	= { .type = NLA_U64 },
 	[NFTA_RULE_USERDATA]	= { .type = NLA_BINARY,
 	[NFTA_RULE_USERDATA]	= { .type = NLA_BINARY,
 				    .len = NFT_USERDATA_MAXLEN },
 				    .len = NFT_USERDATA_MAXLEN },
+	[NFTA_RULE_ID]		= { .type = NLA_U32 },
 };
 };
 
 
 static int nf_tables_fill_rule_info(struct sk_buff *skb, struct net *net,
 static int nf_tables_fill_rule_info(struct sk_buff *skb, struct net *net,
@@ -2446,6 +2505,9 @@ EXPORT_SYMBOL_GPL(nft_unregister_set);
 
 
 static bool nft_set_ops_candidate(const struct nft_set_ops *ops, u32 flags)
 static bool nft_set_ops_candidate(const struct nft_set_ops *ops, u32 flags)
 {
 {
+	if ((flags & NFT_SET_EVAL) && !ops->update)
+		return false;
+
 	return (flags & ops->features) == (flags & NFT_SET_FEATURES);
 	return (flags & ops->features) == (flags & NFT_SET_FEATURES);
 }
 }
 
 
@@ -2510,7 +2572,7 @@ nft_select_set_ops(const struct nft_ctx *ctx,
 				if (est.space == best.space &&
 				if (est.space == best.space &&
 				    est.lookup < best.lookup)
 				    est.lookup < best.lookup)
 					break;
 					break;
-			} else if (est.size < best.size) {
+			} else if (est.size < best.size || !bops) {
 				break;
 				break;
 			}
 			}
 			continue;
 			continue;
@@ -3315,6 +3377,8 @@ static const struct nla_policy nft_set_elem_policy[NFTA_SET_ELEM_MAX + 1] = {
 	[NFTA_SET_ELEM_TIMEOUT]		= { .type = NLA_U64 },
 	[NFTA_SET_ELEM_TIMEOUT]		= { .type = NLA_U64 },
 	[NFTA_SET_ELEM_USERDATA]	= { .type = NLA_BINARY,
 	[NFTA_SET_ELEM_USERDATA]	= { .type = NLA_BINARY,
 					    .len = NFT_USERDATA_MAXLEN },
 					    .len = NFT_USERDATA_MAXLEN },
+	[NFTA_SET_ELEM_EXPR]		= { .type = NLA_NESTED },
+	[NFTA_SET_ELEM_OBJREF]		= { .type = NLA_STRING },
 };
 };
 
 
 static const struct nla_policy nft_set_elem_list_policy[NFTA_SET_ELEM_LIST_MAX + 1] = {
 static const struct nla_policy nft_set_elem_list_policy[NFTA_SET_ELEM_LIST_MAX + 1] = {
@@ -4864,8 +4928,6 @@ nf_tables_flowtable_lookup_byhandle(const struct nft_table *table,
        return ERR_PTR(-ENOENT);
        return ERR_PTR(-ENOENT);
 }
 }
 
 
-#define NFT_FLOWTABLE_DEVICE_MAX	8
-
 static int nf_tables_parse_devices(const struct nft_ctx *ctx,
 static int nf_tables_parse_devices(const struct nft_ctx *ctx,
 				   const struct nlattr *attr,
 				   const struct nlattr *attr,
 				   struct net_device *dev_array[], int *len)
 				   struct net_device *dev_array[], int *len)
@@ -4882,7 +4944,7 @@ static int nf_tables_parse_devices(const struct nft_ctx *ctx,
 		}
 		}
 
 
 		nla_strlcpy(ifname, tmp, IFNAMSIZ);
 		nla_strlcpy(ifname, tmp, IFNAMSIZ);
-		dev = dev_get_by_name(ctx->net, ifname);
+		dev = __dev_get_by_name(ctx->net, ifname);
 		if (!dev) {
 		if (!dev) {
 			err = -ENOENT;
 			err = -ENOENT;
 			goto err1;
 			goto err1;
@@ -4938,13 +5000,11 @@ static int nf_tables_flowtable_parse_hook(const struct nft_ctx *ctx,
 	err = nf_tables_parse_devices(ctx, tb[NFTA_FLOWTABLE_HOOK_DEVS],
 	err = nf_tables_parse_devices(ctx, tb[NFTA_FLOWTABLE_HOOK_DEVS],
 				      dev_array, &n);
 				      dev_array, &n);
 	if (err < 0)
 	if (err < 0)
-		goto err1;
+		return err;
 
 
 	ops = kzalloc(sizeof(struct nf_hook_ops) * n, GFP_KERNEL);
 	ops = kzalloc(sizeof(struct nf_hook_ops) * n, GFP_KERNEL);
-	if (!ops) {
-		err = -ENOMEM;
-		goto err1;
-	}
+	if (!ops)
+		return -ENOMEM;
 
 
 	flowtable->hooknum	= hooknum;
 	flowtable->hooknum	= hooknum;
 	flowtable->priority	= priority;
 	flowtable->priority	= priority;
@@ -4958,13 +5018,10 @@ static int nf_tables_flowtable_parse_hook(const struct nft_ctx *ctx,
 		flowtable->ops[i].priv		= &flowtable->data.rhashtable;
 		flowtable->ops[i].priv		= &flowtable->data.rhashtable;
 		flowtable->ops[i].hook		= flowtable->data.type->hook;
 		flowtable->ops[i].hook		= flowtable->data.type->hook;
 		flowtable->ops[i].dev		= dev_array[i];
 		flowtable->ops[i].dev		= dev_array[i];
+		flowtable->dev_name[i]		= kstrdup(dev_array[i]->name,
+							  GFP_KERNEL);
 	}
 	}
 
 
-	err = 0;
-err1:
-	for (i = 0; i < n; i++)
-		dev_put(dev_array[i]);
-
 	return err;
 	return err;
 }
 }
 
 
@@ -5135,8 +5192,10 @@ static int nf_tables_newflowtable(struct net *net, struct sock *nlsk,
 err5:
 err5:
 	i = flowtable->ops_len;
 	i = flowtable->ops_len;
 err4:
 err4:
-	for (k = i - 1; k >= 0; k--)
+	for (k = i - 1; k >= 0; k--) {
+		kfree(flowtable->dev_name[k]);
 		nf_unregister_net_hook(net, &flowtable->ops[k]);
 		nf_unregister_net_hook(net, &flowtable->ops[k]);
+	}
 
 
 	kfree(flowtable->ops);
 	kfree(flowtable->ops);
 err3:
 err3:
@@ -5226,9 +5285,9 @@ static int nf_tables_fill_flowtable_info(struct sk_buff *skb, struct net *net,
 		goto nla_put_failure;
 		goto nla_put_failure;
 
 
 	for (i = 0; i < flowtable->ops_len; i++) {
 	for (i = 0; i < flowtable->ops_len; i++) {
-		if (flowtable->ops[i].dev &&
+		if (flowtable->dev_name[i][0] &&
 		    nla_put_string(skb, NFTA_DEVICE_NAME,
 		    nla_put_string(skb, NFTA_DEVICE_NAME,
-				   flowtable->ops[i].dev->name))
+				   flowtable->dev_name[i]))
 			goto nla_put_failure;
 			goto nla_put_failure;
 	}
 	}
 	nla_nest_end(skb, nest_devs);
 	nla_nest_end(skb, nest_devs);
@@ -5470,6 +5529,7 @@ static void nft_flowtable_event(unsigned long event, struct net_device *dev,
 			continue;
 			continue;
 
 
 		nf_unregister_net_hook(dev_net(dev), &flowtable->ops[i]);
 		nf_unregister_net_hook(dev_net(dev), &flowtable->ops[i]);
+		flowtable->dev_name[i][0] = '\0';
 		flowtable->ops[i].dev = NULL;
 		flowtable->ops[i].dev = NULL;
 		break;
 		break;
 	}
 	}

+ 1 - 1
net/netfilter/nft_set_hash.c

@@ -674,7 +674,7 @@ static const struct nft_set_ops *
 nft_hash_select_ops(const struct nft_ctx *ctx, const struct nft_set_desc *desc,
 nft_hash_select_ops(const struct nft_ctx *ctx, const struct nft_set_desc *desc,
 		    u32 flags)
 		    u32 flags)
 {
 {
-	if (desc->size && !(flags & NFT_SET_TIMEOUT)) {
+	if (desc->size && !(flags & (NFT_SET_EVAL | NFT_SET_TIMEOUT))) {
 		switch (desc->klen) {
 		switch (desc->klen) {
 		case 4:
 		case 4:
 			return &nft_hash_fast_ops;
 			return &nft_hash_fast_ops;