Netfilter/IPVS fixes for net

PATCH In review Pablo Neira Ayuso 2 participants 16 messages Mar 2, 2018 – Mar 3, 2018

Subsystems (5)

Networking Network drivers netfilter Ethernet bridge +1 more subsystems

lore

Share & tools

lore

Pablo Neira Ayuso

author maintainer

2018-03-02 20:33 UTC #0 lore raw

cover Netfilter/IPVS fixes for net

Hi David,

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

1) Put back reference on CLUSTERIP configuration structure from the error path, patch from Florian Westphal.

2) Put reference on CLUSTERIP configuration instead of freeing it, another cpu may still be walking over it, also from Florian.

3) Refetch pointer to IPv6 header from nf_nat_ipv6_manip_pkt() given packet manipulation may reallocation the skbuff header, from Florian.

4) Missing match size sanity checks in ebt_among, from Florian.

5) Convert BUG_ON to WARN_ON in ebtables, from Florian.

6) Sanity check userspace offsets from ebtables kernel, from Florian.

7) Missing checksum replace call in flowtable IPv4 DNAT, from Felix Fietkau.

8) Bump the right stats on checksum error from bridge netfilter, from Taehee Yoo.

9) Unset interface flag in IPv6 fib lookups otherwise we get misleading routing lookup results, from Florian.

10) Missing sk_to_full_sk() in ip6_route_me_harder() from Eric Dumazet.

11) Don't allow devices to be part of multiple flowtables at the same time, this may break setups.

12) Missing netlink attribute validation in flowtable deletion.

13) Wrong array index in nf_unregister_net_hook() call from error path in flowtable addition path.

14) Fix FTP IPVS helper when NAT mangling is in place, patch from Julian Anastasov.

You can pull these changes from:

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

Thanks!

----------------------------------------------------------------

The following changes since commit 9cb9c07d6b0c5fd97d83b8ab14d7e308ba4b612f:

Merge git://git.kernel.org/pub/scm/linux/kernel/git/davem/net (2018-02-23 15:14:17 -0800)

are available in the git repository at:

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

for you to fetch changes up to 8a949fff0302b50063f74bb345a66190015528d0:

ipvs: remove IPS_NAT_MASK check to fix passive FTP (2018-02-28 19:48:26 +0100)

----------------------------------------------------------------
Eric Dumazet (1):
netfilter: use skb_to_full_sk in ip6_route_me_harder

Felix Fietkau (1):
netfilter: nf_flow_table: fix checksum when handling DNAT

Florian Westphal (7):
netfilter: ipt_CLUSTERIP: put config struct if we can't increment ct refcount
netfilter: ipt_CLUSTERIP: put config instead of freeing it
netfilter: ipv6: fix use-after-free Write in nf_nat_ipv6_manip_pkt
netfilter: bridge: ebt_among: add missing match size checks
netfilter: ebtables: convert BUG_ONs to WARN_ONs
netfilter: ebtables: CONFIG_COMPAT: don't trust userland offsets
netfilter: don't set F_IFACE on ipv6 fib lookups

Julian Anastasov (1):
ipvs: remove IPS_NAT_MASK check to fix passive FTP

Pablo Neira Ayuso (3):
netfilter: nf_tables: return EBUSY if device already belongs to flowtable
netfilter: nf_tables: missing attribute validation in nf_tables_delflowtable()
netfilter: nf_tables: use the right index from flowtable error path

Taehee Yoo (1):
netfilter: increase IPSTATS_MIB_CSUMERRORS stat

 net/bridge/br_netfilter_hooks.c          |  4 +++-
 net/bridge/netfilter/ebt_among.c         | 21 +++++++++++++++--
 net/bridge/netfilter/ebtables.c          | 40 ++++++++++++++++++++++++--------
 net/ipv4/netfilter/ipt_CLUSTERIP.c       | 15 ++++++++----
 net/ipv4/netfilter/nf_flow_table_ipv4.c  |  1 +
 net/ipv6/netfilter.c                     |  9 +++----
 net/ipv6/netfilter/ip6t_rpfilter.c       |  4 ----
 net/ipv6/netfilter/nf_nat_l3proto_ipv6.c |  4 ++++
 net/ipv6/netfilter/nft_fib_ipv6.c        | 12 ++--------
 net/netfilter/ipvs/ip_vs_ftp.c           |  2 +-
 net/netfilter/nf_tables_api.c            | 25 ++++++++++++++++++--
 11 files changed, 98 insertions(+), 39 deletions(-)

Pablo Neira Ayuso

author maintainer

2018-03-02 20:33 UTC #1 lore raw

PATCH 1/14 netfilter: ipt_CLUSTERIP: put config struct if we can't increment ct refcount

From: Florian Westphal <[email protected]>

This needs to put() the entry to avoid a resource leak in error path.

Signed-off-by Florian Westphal <[email protected]>

Signed-off-by Pablo Neira Ayuso <[email protected]>

---
 net/ipv4/netfilter/ipt_CLUSTERIP.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

net/ipv4/netfilter/ipt_CLUSTERIP.c +6 -2

index 4b02ab39ebc5..4c8cfd352687 100644
@@ -496,12 +496,15 @@ static int clusterip_tg_check(const struct xt_tgchk_param *par)
 				return PTR_ERR(config);
 		}
 	}
-	cipinfo->config = config;
 
 	ret = nf_ct_netns_get(par->net, par->family);
-	if (ret < 0)
+	if (ret < 0) {
 		pr_info("cannot load conntrack support for proto=%u\n",
 			par->family);
+		clusterip_config_entry_put(par->net, config);
+		clusterip_config_put(config);
+		return ret;
+	}
 
 	if (!par->net->xt.clusterip_deprecated_warning) {
 		pr_info("ipt_CLUSTERIP is deprecated and it will removed soon, "

@@ -509,6 +512,7 @@ static int clusterip_tg_check(const struct xt_tgchk_param *par)
 		par->net->xt.clusterip_deprecated_warning = true;
 	}
 
+	cipinfo->config = config;
 	return ret;
 }

Signature · 1 line

2.11.0

Pablo Neira Ayuso

author maintainer

2018-03-02 20:33 UTC #2 lore raw

PATCH 2/14 netfilter: ipt_CLUSTERIP: put config instead of freeing it

In reply to Pablo Neira Ayuso

From: Florian Westphal <[email protected]>

Once struct is added to per-netns list it becomes visible to other cpus, so we cannot use kfree().

Also delay setting entries refcount to 1 until after everything is initialised so that when we call clusterip_config_put() in this spot entries is still zero.

Signed-off-by Florian Westphal <[email protected]>

Signed-off-by Pablo Neira Ayuso <[email protected]>

---
 net/ipv4/netfilter/ipt_CLUSTERIP.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

net/ipv4/netfilter/ipt_CLUSTERIP.c +4 -3

index 4c8cfd352687..8a8ae61cea71 100644
@@ -232,7 +232,6 @@ clusterip_config_init(struct net *net, const struct ipt_clusterip_tgt_info *i,
 	c->hash_mode = i->hash_mode;
 	c->hash_initval = i->hash_initval;
 	refcount_set(&c->refcount, 1);
-	refcount_set(&c->entries, 1);
 
 	spin_lock_bh(&cn->lock);
 	if (__clusterip_config_find(net, ip)) {

@@ -263,8 +262,10 @@ clusterip_config_init(struct net *net, const struct ipt_clusterip_tgt_info *i,
 
 	c->notifier.notifier_call = clusterip_netdev_event;
 	err = register_netdevice_notifier(&c->notifier);
-	if (!err)
+	if (!err) {
+		refcount_set(&c->entries, 1);
 		return c;
+	}
 
 #ifdef CONFIG_PROC_FS
 	proc_remove(c->pde);

@@ -273,7 +274,7 @@ clusterip_config_init(struct net *net, const struct ipt_clusterip_tgt_info *i,
 	spin_lock_bh(&cn->lock);
 	list_del_rcu(&c->list);
 	spin_unlock_bh(&cn->lock);
-	kfree(c);
+	clusterip_config_put(c);
 
 	return ERR_PTR(err);
 }

Signature · 1 line

2.11.0

Pablo Neira Ayuso

author maintainer

2018-03-02 20:33 UTC #3 lore raw

PATCH 3/14 netfilter: ipv6: fix use-after-free Write in nf_nat_ipv6_manip_pkt

In reply to Pablo Neira Ayuso

From: Florian Westphal <[email protected]>

l4proto->manip_pkt() can cause reallocation of skb head so pointer to the ipv6 header must be reloaded.

Reported-and-tested-by <[email protected]>

Fixes 58a317f1061c89 ("netfilter: ipv6: add IPv6 NAT support")

Signed-off-by Florian Westphal <[email protected]>

Signed-off-by Pablo Neira Ayuso <[email protected]>

---
 net/ipv6/netfilter/nf_nat_l3proto_ipv6.c | 4 ++++
 1 file changed, 4 insertions(+)

net/ipv6/netfilter/nf_nat_l3proto_ipv6.c +4 -0

index bed57ee65f7b..6b7f075f811f 100644
@@ -99,6 +99,10 @@ static bool nf_nat_ipv6_manip_pkt(struct sk_buff *skb,
 	    !l4proto->manip_pkt(skb, &nf_nat_l3proto_ipv6, iphdroff, hdroff,
 				target, maniptype))
 		return false;
+
+	/* must reload, offset might have changed */
+	ipv6h = (void *)skb->data + iphdroff;
+
 manip_addr:
 	if (maniptype == NF_NAT_MANIP_SRC)
 		ipv6h->saddr = target->src.u3.in6;

Signature · 1 line

2.11.0

Pablo Neira Ayuso

author maintainer

2018-03-02 20:33 UTC #4 lore raw

PATCH 4/14 netfilter: bridge: ebt_among: add missing match size checks

In reply to Pablo Neira Ayuso

From: Florian Westphal <[email protected]>

ebt_among is special, it has a dynamic match size and is exempt from the central size checks.

Therefore it must check that the size of the match structure provided from userspace is sane by making sure em->match_size is at least the minimum size of the expected structure.

The module has such a check, but its only done after accessing a structure that might be out of bounds.

tested with: ebtables -A INPUT ... \
--among-dst fe:fe:fe:fe:fe:fe
--among-dst fe:fe:fe:fe:fe:fe --among-src fe:fe:fe:fe:ff:f,fe:fe:fe:fe:fe:fb,fe:fe:fe:fe:fc:fd,fe:fe:fe:fe:fe:fd,fe:fe:fe:fe:fe:fe --among-src fe:fe:fe:fe:ff:f,fe:fe:fe:fe:fe:fa,fe:fe:fe:fe:fe:fd,fe:fe:fe:fe:fe:fe,fe:fe:fe:fe:fe:fe

Reported-by <[email protected]>

Signed-off-by Florian Westphal <[email protected]>

Signed-off-by Pablo Neira Ayuso <[email protected]>

---
 net/bridge/netfilter/ebt_among.c | 21 +++++++++++++++++++--
 1 file changed, 19 insertions(+), 2 deletions(-)

net/bridge/netfilter/ebt_among.c +19 -2

index ce7152a12bd8..c5afb4232ecb 100644
@@ -172,18 +172,35 @@ ebt_among_mt(const struct sk_buff *skb, struct xt_action_param *par)
 	return true;
 }
 
+static bool poolsize_invalid(const struct ebt_mac_wormhash *w)
+{
+	return w && w->poolsize >= (INT_MAX / sizeof(struct ebt_mac_wormhash_tuple));
+}
+
 static int ebt_among_mt_check(const struct xt_mtchk_param *par)
 {
 	const struct ebt_among_info *info = par->matchinfo;
 	const struct ebt_entry_match *em =
 		container_of(par->matchinfo, const struct ebt_entry_match, data);
-	int expected_length = sizeof(struct ebt_among_info);
+	unsigned int expected_length = sizeof(struct ebt_among_info);
 	const struct ebt_mac_wormhash *wh_dst, *wh_src;
 	int err;
 
+	if (expected_length > em->match_size)
+		return -EINVAL;
+
 	wh_dst = ebt_among_wh_dst(info);
-	wh_src = ebt_among_wh_src(info);
+	if (poolsize_invalid(wh_dst))
+		return -EINVAL;
+
 	expected_length += ebt_mac_wormhash_size(wh_dst);
+	if (expected_length > em->match_size)
+		return -EINVAL;
+
+	wh_src = ebt_among_wh_src(info);
+	if (poolsize_invalid(wh_src))
+		return -EINVAL;
+
 	expected_length += ebt_mac_wormhash_size(wh_src);
 
 	if (em->match_size != EBT_ALIGN(expected_length)) {

Signature · 1 line

2.11.0

Pablo Neira Ayuso

author maintainer

2018-03-02 20:33 UTC #5 lore raw

PATCH 5/14 netfilter: ebtables: convert BUG_ONs to WARN_ONs

In reply to Pablo Neira Ayuso

From: Florian Westphal <[email protected]>

All of these conditions are not fatal and should have been WARN_ONs from the get-go.

Convert them to WARN_ONs and bail out.

Signed-off-by Florian Westphal <[email protected]>

Signed-off-by Pablo Neira Ayuso <[email protected]>

---
 net/bridge/netfilter/ebtables.c | 27 ++++++++++++++++++---------
 1 file changed, 18 insertions(+), 9 deletions(-)

net/bridge/netfilter/ebtables.c +18 -9

index 02c4b409d317..61f87879e389 100644
@@ -1641,7 +1641,8 @@ static int compat_match_to_user(struct ebt_entry_match *m, void __user **dstptr,
 	int off = ebt_compat_match_offset(match, m->match_size);
 	compat_uint_t msize = m->match_size - off;
 
-	BUG_ON(off >= m->match_size);
+	if (WARN_ON(off >= m->match_size))
+		return -EINVAL;
 
 	if (copy_to_user(cm->u.name, match->name,
 	    strlen(match->name) + 1) || put_user(msize, &cm->match_size))

@@ -1671,7 +1672,8 @@ static int compat_target_to_user(struct ebt_entry_target *t,
 	int off = xt_compat_target_offset(target);
 	compat_uint_t tsize = t->target_size - off;
 
-	BUG_ON(off >= t->target_size);
+	if (WARN_ON(off >= t->target_size))
+		return -EINVAL;
 
 	if (copy_to_user(cm->u.name, target->name,
 	    strlen(target->name) + 1) || put_user(tsize, &cm->match_size))

@@ -1902,7 +1904,8 @@ static int ebt_buf_add(struct ebt_entries_buf_state *state,
 	if (state->buf_kern_start == NULL)
 		goto count_only;
 
-	BUG_ON(state->buf_kern_offset + sz > state->buf_kern_len);
+	if (WARN_ON(state->buf_kern_offset + sz > state->buf_kern_len))
+		return -EINVAL;
 
 	memcpy(state->buf_kern_start + state->buf_kern_offset, data, sz);

@@ -1915,7 +1918,8 @@ static int ebt_buf_add_pad(struct ebt_entries_buf_state *state, unsigned int sz)
 {
 	char *b = state->buf_kern_start;
 
-	BUG_ON(b && state->buf_kern_offset > state->buf_kern_len);
+	if (WARN_ON(b && state->buf_kern_offset > state->buf_kern_len))
+		return -EINVAL;
 
 	if (b != NULL && sz > 0)
 		memset(b + state->buf_kern_offset, 0, sz);

@@ -1992,8 +1996,10 @@ static int compat_mtw_from_user(struct compat_ebt_entry_mwt *mwt,
 	pad = XT_ALIGN(size_kern) - size_kern;
 
 	if (pad > 0 && dst) {
-		BUG_ON(state->buf_kern_len <= pad);
-		BUG_ON(state->buf_kern_offset - (match_size + off) + size_kern > state->buf_kern_len - pad);
+		if (WARN_ON(state->buf_kern_len <= pad))
+			return -EINVAL;
+		if (WARN_ON(state->buf_kern_offset - (match_size + off) + size_kern > state->buf_kern_len - pad))
+			return -EINVAL;
 		memset(dst + size_kern, 0, pad);
 	}
 	return off + match_size;

@@ -2043,7 +2049,8 @@ static int ebt_size_mwt(struct compat_ebt_entry_mwt *match32,
 		if (ret < 0)
 			return ret;
 
-		BUG_ON(ret < match32->match_size);
+		if (WARN_ON(ret < match32->match_size))
+			return -EINVAL;
 		growth += ret - match32->match_size;
 		growth += ebt_compat_entry_padsize();

@@ -2140,7 +2147,8 @@ static int size_entry_mwt(struct ebt_entry *entry, const unsigned char *base,
 
 	startoff = state->buf_user_offset - startoff;
 
-	BUG_ON(*total < startoff);
+	if (WARN_ON(*total < startoff))
+		return -EINVAL;
 	*total -= startoff;
 	return 0;
 }

@@ -2267,7 +2275,8 @@ static int compat_do_replace(struct net *net, void __user *user,
 	state.buf_kern_len = size64;
 
 	ret = compat_copy_entries(entries_tmp, tmp.entries_size, &state);
-	BUG_ON(ret < 0);	/* parses same data again */
+	if (WARN_ON(ret < 0))
+		goto out_unlock;
 
 	vfree(entries_tmp);
 	tmp.entries_size = size64;

Signature · 1 line

2.11.0

Pablo Neira Ayuso

author maintainer

2018-03-02 20:33 UTC #6 lore raw

PATCH 6/14 netfilter: ebtables: CONFIG_COMPAT: don't trust userland offsets

In reply to Pablo Neira Ayuso

From: Florian Westphal <[email protected]>

We need to make sure the offsets are not out of range of the total size.
Also check that they are in ascending order.

The WARN_ON triggered by syzkaller (it sets panic_on_warn) is changed to also bail out, no point in continuing parsing.

Briefly tested with simple ruleset of
-A INPUT --limit 1/s' --log
plus jump to custom chains using 32bit ebtables binary.

Reported-by <[email protected]>

Signed-off-by Florian Westphal <[email protected]>

Signed-off-by Pablo Neira Ayuso <[email protected]>

---
 net/bridge/netfilter/ebtables.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

net/bridge/netfilter/ebtables.c +12 -1

index 61f87879e389..254ef9f49567 100644
@@ -2060,7 +2060,9 @@ static int ebt_size_mwt(struct compat_ebt_entry_mwt *match32,
 		if (match_kern)
 			match_kern->match_size = ret;
 
-		WARN_ON(type == EBT_COMPAT_TARGET && size_left);
+		if (WARN_ON(type == EBT_COMPAT_TARGET && size_left))
+			return -EINVAL;
+
 		match32 = (struct compat_ebt_entry_mwt *) buf;
 	}

@@ -2116,6 +2118,15 @@ static int size_entry_mwt(struct ebt_entry *entry, const unsigned char *base,
 	 *
 	 * offsets are relative to beginning of struct ebt_entry (i.e., 0).
 	 */
+	for (i = 0; i < 4 ; ++i) {
+		if (offsets[i] >= *total)
+			return -EINVAL;
+		if (i == 0)
+			continue;
+		if (offsets[i-1] > offsets[i])
+			return -EINVAL;
+	}
+
 	for (i = 0, j = 1 ; j < 4 ; j++, i++) {
 		struct compat_ebt_entry_mwt *match32;
 		unsigned int size;

Signature · 1 line

2.11.0

Pablo Neira Ayuso

author maintainer

2018-03-02 20:33 UTC #7 lore raw

PATCH 7/14 netfilter: nf_flow_table: fix checksum when handling DNAT

In reply to Pablo Neira Ayuso

From: Felix Fietkau <[email protected]>

Add a missing call to csum_replace4 like on SNAT.

Signed-off-by Felix Fietkau <[email protected]>

Signed-off-by Pablo Neira Ayuso <[email protected]>

---
 net/ipv4/netfilter/nf_flow_table_ipv4.c | 1 +
 1 file changed, 1 insertion(+)

net/ipv4/netfilter/nf_flow_table_ipv4.c +1 -0

index 25d2975da156..282b9cc4fe82 100644
@@ -111,6 +111,7 @@ static int nf_flow_dnat_ip(const struct flow_offload *flow, struct sk_buff *skb,
 	default:
 		return -1;
 	}
+	csum_replace4(&iph->check, addr, new_addr);
 
 	return nf_flow_nat_ip_l4proto(skb, iph, thoff, addr, new_addr);
 }

Signature · 1 line

2.11.0

Pablo Neira Ayuso

author maintainer

2018-03-02 20:33 UTC #8 lore raw

PATCH 8/14 netfilter: increase IPSTATS_MIB_CSUMERRORS stat

In reply to Pablo Neira Ayuso

From: Taehee Yoo <[email protected]>

In the ip_rcv, IPSTATS_MIB_CSUMERRORS is increased when checksum error is occurred.
bridge netfilter routine should increase IPSTATS_MIB_CSUMERRORS.

Signed-off-by Taehee Yoo <[email protected]>

Signed-off-by Pablo Neira Ayuso <[email protected]>

---
 net/bridge/br_netfilter_hooks.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

net/bridge/br_netfilter_hooks.c +3 -1

index 27f1d4f2114a..9b16eaf33819 100644
@@ -214,7 +214,7 @@ static int br_validate_ipv4(struct net *net, struct sk_buff *skb)
 
 	iph = ip_hdr(skb);
 	if (unlikely(ip_fast_csum((u8 *)iph, iph->ihl)))
-		goto inhdr_error;
+		goto csum_error;
 
 	len = ntohs(iph->tot_len);
 	if (skb->len < len) {

@@ -236,6 +236,8 @@ static int br_validate_ipv4(struct net *net, struct sk_buff *skb)
 	 */
 	return 0;
 
+csum_error:
+	__IP_INC_STATS(net, IPSTATS_MIB_CSUMERRORS);
 inhdr_error:
 	__IP_INC_STATS(net, IPSTATS_MIB_INHDRERRORS);
 drop:

Signature · 1 line

2.11.0

Pablo Neira Ayuso

author maintainer

2018-03-02 20:33 UTC #9 lore raw

PATCH 9/14 netfilter: don't set F_IFACE on ipv6 fib lookups

In reply to Pablo Neira Ayuso

From: Florian Westphal <[email protected]>

"fib" starts to behave strangely when an ipv6 default route is added - the FIB lookup returns a route using 'oif' in this case.

This behaviour was inherited from ip6tables rpfilter so change this as well.

Bugzilla: https://bugzilla.netfilter.org/show_bug.cgi?id=1221

Signed-off-by Florian Westphal <[email protected]>

Signed-off-by Pablo Neira Ayuso <[email protected]>

---
 net/ipv6/netfilter/ip6t_rpfilter.c |  4 ----
 net/ipv6/netfilter/nft_fib_ipv6.c  | 12 ++----------
 2 files changed, 2 insertions(+), 14 deletions(-)

net/ipv6/netfilter/ip6t_rpfilter.c +0 -4

index 94deb69bbbda..91ed25a24b79 100644
@@ -48,10 +48,6 @@ static bool rpfilter_lookup_reverse6(struct net *net, const struct sk_buff *skb,
 	}
 
 	fl6.flowi6_mark = flags & XT_RPFILTER_VALID_MARK ? skb->mark : 0;
-	if ((flags & XT_RPFILTER_LOOSE) == 0) {
-		fl6.flowi6_oif = dev->ifindex;
-		lookup_flags |= RT6_LOOKUP_F_IFACE;
-	}
 
 	rt = (void *) ip6_route_lookup(net, &fl6, lookup_flags);
 	if (rt->dst.error)

net/ipv6/netfilter/nft_fib_ipv6.c +2 ~~-10~~

index cc5174c7254c..62fc84d7bdff 100644
@@ -180,7 +180,6 @@ void nft_fib6_eval(const struct nft_expr *expr, struct nft_regs *regs,
 	}
 
 	*dest = 0;
- again:
 	rt = (void *)ip6_route_lookup(nft_net(pkt), &fl6, lookup_flags);
 	if (rt->dst.error)
 		goto put_rt_err;

@@ -189,15 +188,8 @@ void nft_fib6_eval(const struct nft_expr *expr, struct nft_regs *regs,
 	if (rt->rt6i_flags & (RTF_REJECT | RTF_ANYCAST | RTF_LOCAL))
 		goto put_rt_err;
 
-	if (oif && oif != rt->rt6i_idev->dev) {
-		/* multipath route? Try again with F_IFACE */
-		if ((lookup_flags & RT6_LOOKUP_F_IFACE) == 0) {
-			lookup_flags |= RT6_LOOKUP_F_IFACE;
-			fl6.flowi6_oif = oif->ifindex;
-			ip6_rt_put(rt);
-			goto again;
-		}
-	}
+	if (oif && oif != rt->rt6i_idev->dev)
+		goto put_rt_err;
 
 	switch (priv->result) {
 	case NFT_FIB_RESULT_OIF:

Signature · 1 line

2.11.0

Pablo Neira Ayuso

author maintainer

2018-03-02 20:33 UTC #10 lore raw

PATCH 10/14 netfilter: use skb_to_full_sk in ip6_route_me_harder

In reply to Pablo Neira Ayuso

From: Eric Dumazet <[email protected]>

For some reason, Florian forgot to apply to ip6_route_me_harder the fix that went in commit 29e09229d9f2 ("netfilter: use skb_to_full_sk in ip_route_me_harder")

Fixes ca6fb0651883 ("tcp: attach SYNACK messages to request sockets instead of listener")

Signed-off-by Eric Dumazet <[email protected]>

Reported-by syzbot <[email protected]>

Signed-off-by Pablo Neira Ayuso <[email protected]>

---
 net/ipv6/netfilter.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

net/ipv6/netfilter.c +5 -4

index d95ceca7ff8f..531d6957af36 100644
@@ -21,18 +21,19 @@
 int ip6_route_me_harder(struct net *net, struct sk_buff *skb)
 {
 	const struct ipv6hdr *iph = ipv6_hdr(skb);
+	struct sock *sk = sk_to_full_sk(skb->sk);
 	unsigned int hh_len;
 	struct dst_entry *dst;
 	struct flowi6 fl6 = {
-		.flowi6_oif = skb->sk ? skb->sk->sk_bound_dev_if : 0,
+		.flowi6_oif = sk ? sk->sk_bound_dev_if : 0,
 		.flowi6_mark = skb->mark,
-		.flowi6_uid = sock_net_uid(net, skb->sk),
+		.flowi6_uid = sock_net_uid(net, sk),
 		.daddr = iph->daddr,
 		.saddr = iph->saddr,
 	};
 	int err;
 
-	dst = ip6_route_output(net, skb->sk, &fl6);
+	dst = ip6_route_output(net, sk, &fl6);
 	err = dst->error;
 	if (err) {
 		IP6_INC_STATS(net, ip6_dst_idev(dst), IPSTATS_MIB_OUTNOROUTES);

@@ -50,7 +51,7 @@ int ip6_route_me_harder(struct net *net, struct sk_buff *skb)
 	if (!(IP6CB(skb)->flags & IP6SKB_XFRM_TRANSFORMED) &&
 	    xfrm_decode_session(skb, flowi6_to_flowi(&fl6), AF_INET6) == 0) {
 		skb_dst_set(skb, NULL);
-		dst = xfrm_lookup(net, dst, flowi6_to_flowi(&fl6), skb->sk, 0);
+		dst = xfrm_lookup(net, dst, flowi6_to_flowi(&fl6), sk, 0);
 		if (IS_ERR(dst))
 			return PTR_ERR(dst);
 		skb_dst_set(skb, dst);

Signature · 1 line

2.11.0

Pablo Neira Ayuso

author maintainer

2018-03-02 20:33 UTC #11 lore raw

PATCH 11/14 netfilter: nf_tables: return EBUSY if device already belongs to flowtable

In reply to Pablo Neira Ayuso

If the netdevice is already part of a flowtable, return EBUSY. I cannot find a valid usecase for having two flowtables bound to the same netdevice. We can still have two flowtable where the device set is disjoint.

Signed-off-by Pablo Neira Ayuso <[email protected]>

---
 net/netfilter/nf_tables_api.c | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

net/netfilter/nf_tables_api.c +17 -1

index 8b9fe30de0cd..43acdeef045d 100644
@@ -5037,9 +5037,9 @@ static int nf_tables_newflowtable(struct net *net, struct sock *nlsk,
 {
 	const struct nfgenmsg *nfmsg = nlmsg_data(nlh);
 	const struct nf_flowtable_type *type;
+	struct nft_flowtable *flowtable, *ft;
 	u8 genmask = nft_genmask_next(net);
 	int family = nfmsg->nfgen_family;
-	struct nft_flowtable *flowtable;
 	struct nft_table *table;
 	struct nft_ctx ctx;
 	int err, i, k;

@@ -5099,6 +5099,22 @@ static int nf_tables_newflowtable(struct net *net, struct sock *nlsk,
 		goto err3;
 
 	for (i = 0; i < flowtable->ops_len; i++) {
+		if (!flowtable->ops[i].dev)
+			continue;
+
+		list_for_each_entry(ft, &table->flowtables, list) {
+			for (k = 0; k < ft->ops_len; k++) {
+				if (!ft->ops[k].dev)
+					continue;
+
+				if (flowtable->ops[i].dev == ft->ops[k].dev &&
+				    flowtable->ops[i].pf == ft->ops[k].pf) {
+					err = -EBUSY;
+					goto err4;
+				}
+			}
+		}
+
 		err = nf_register_net_hook(net, &flowtable->ops[i]);
 		if (err < 0)
 			goto err4;

Signature · 1 line

2.11.0

Pablo Neira Ayuso

author maintainer

2018-03-02 20:33 UTC #12 lore raw

PATCH 12/14 netfilter: nf_tables: missing attribute validation in nf_tables_delflowtable()

In reply to Pablo Neira Ayuso

Return -EINVAL is mandatory attributes are missing.

Signed-off-by Pablo Neira Ayuso <[email protected]>

---
 net/netfilter/nf_tables_api.c | 5 +++++
 1 file changed, 5 insertions(+)

net/netfilter/nf_tables_api.c +5 -0

index 43acdeef045d..2b5aa78979db 100644
@@ -5161,6 +5161,11 @@ static int nf_tables_delflowtable(struct net *net, struct sock *nlsk,
 	struct nft_table *table;
 	struct nft_ctx ctx;
 
+	if (!nla[NFTA_FLOWTABLE_TABLE] ||
+	    (!nla[NFTA_FLOWTABLE_NAME] &&
+	     !nla[NFTA_FLOWTABLE_HANDLE]))
+		return -EINVAL;
+
 	table = nf_tables_table_lookup(net, nla[NFTA_FLOWTABLE_TABLE],
 				       family, genmask);
 	if (IS_ERR(table))

Signature · 1 line

2.11.0

Pablo Neira Ayuso

author maintainer

2018-03-02 20:33 UTC #13 lore raw

PATCH 13/14 netfilter: nf_tables: use the right index from flowtable error path

In reply to Pablo Neira Ayuso

Use the right loop index, not the number of devices in the array that we need to remove, the following message uncovered the problem:

[ 5437.044119] hook not found, pf 5 num 0
[ 5437.044140] WARNING: CPU: 2 PID: 24983 at net/netfilter/core.c:376 __nf_unregister_net_hook+0x250/0x280

Signed-off-by Pablo Neira Ayuso <[email protected]>

---
 net/netfilter/nf_tables_api.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

net/netfilter/nf_tables_api.c +1 -1

index 2b5aa78979db..558593e6a0a3 100644
@@ -5136,7 +5136,7 @@ static int nf_tables_newflowtable(struct net *net, struct sock *nlsk,
 	i = flowtable->ops_len;
 err4:
 	for (k = i - 1; k >= 0; k--)
-		nf_unregister_net_hook(net, &flowtable->ops[i]);
+		nf_unregister_net_hook(net, &flowtable->ops[k]);
 
 	kfree(flowtable->ops);
 err3:

Signature · 1 line

2.11.0

Pablo Neira Ayuso

author maintainer

2018-03-02 20:33 UTC #14 lore raw

PATCH 14/14 ipvs: remove IPS_NAT_MASK check to fix passive FTP

In reply to Pablo Neira Ayuso

From: Julian Anastasov <[email protected]>

The IPS_NAT_MASK check in 4.12 replaced previous check for nfct_nat() which was needed to fix a crash in 2.6.36-rc, see
commit 7bcbf81a2296 ("ipvs: avoid oops for passive FTP"). But as IPVS does not set the IPS_SRC_NAT and IPS_DST_NAT bits, checking for IPS_NAT_MASK prevents PASV response to be properly mangled and blocks the transfer. Remove the check as it is not needed after 3.12 commit 41d73ec053d2 ("netfilter: nf_conntrack:
make sequence number adjustments usuable without NAT") which changes nfct_nat() with nfct_seqadj() and especially after 3.13 commit b25adce16064 ("ipvs: correct usage/allocation of seqadj ext in ipvs").

Thanks to Li Shuang and Florian Westphal for reporting the problem!

Reported-by Li Shuang <[email protected]>

Fixes be7be6e161a2 ("netfilter: ipvs: fix incorrect conflict resolution")

Signed-off-by Julian Anastasov <[email protected]>

Acked-by Simon Horman <[email protected]>

Signed-off-by Pablo Neira Ayuso <[email protected]>

---
 net/netfilter/ipvs/ip_vs_ftp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

net/netfilter/ipvs/ip_vs_ftp.c +1 -1

index 3e17d32b629d..58d5d05aec24 100644
@@ -260,7 +260,7 @@ static int ip_vs_ftp_out(struct ip_vs_app *app, struct ip_vs_conn *cp,
 		buf_len = strlen(buf);
 
 		ct = nf_ct_get(skb, &ctinfo);
-		if (ct && (ct->status & IPS_NAT_MASK)) {
+		if (ct) {
 			bool mangled;
 
 			/* If mangling fails this function will return 0

Signature · 1 line

2.11.0

David Miller

maintainer

2018-03-03 01:32 UTC #15 lore raw

In reply to Pablo Neira Ayuso

From: Pablo Neira Ayuso <[email protected]>
Date: Fri, 2 Mar 2018 21:32:48 +0100

Quoted 2 lines

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

...

Quoted 3 lines

You can pull these changes from:
git://git.kernel.org/pub/scm/linux/kernel/git/pablo/nf.git

Pulled, thank you.