[PATCH 0/5] Netfilter fixes for net

[PATCH 0/5] Netfilter fixes for net

[Pablo Neira Ayuso]

Hi,

The following patchset contains Netfilter fixes for net:

1) Set VLAN tag in tcp reset/icmp unreachable packets to reject
   connections in the bridge family, from Michael Braun.

2) Incorrect subcounter flag update in ipset, from Phil Sutter.

3) Possible buffer overflow in the pptp conntrack helper, based
   on patch from Dan Carpenter.

4) Restore userspace conntrack helper hook logic that broke after
   hook consolidation rework.

5) Unbreak userspace conntrack helper registration via
   nfnetlink_cthelper.

You can pull these changes from:

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

Thank you.

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

The following changes since commit 98790bbac4db1697212ce9462ec35ca09c4a2810:

  Merge tag 'efi-urgent-2020-05-24' of git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip (2020-05-24 10:24:10 -0700)

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 703acd70f2496537457186211c2f03e792409e68:

  netfilter: nfnetlink_cthelper: unbreak userspace helper support (2020-05-25 20:39:14 +0200)

----------------------------------------------------------------
Michael Braun (1):
      netfilter: nft_reject_bridge: enable reject with bridge vlan

Pablo Neira Ayuso (3):
      netfilter: nf_conntrack_pptp: prevent buffer overflows in debug code
      netfilter: conntrack: make conntrack userspace helpers work again
      netfilter: nfnetlink_cthelper: unbreak userspace helper support

Phil Sutter (1):
      netfilter: ipset: Fix subcounter update skip

 include/linux/netfilter/nf_conntrack_pptp.h |  2 +-
 net/bridge/netfilter/nft_reject_bridge.c    |  6 +++
 net/ipv4/netfilter/nf_nat_pptp.c            |  7 +--
 net/netfilter/ipset/ip_set_list_set.c       |  2 +-
 net/netfilter/nf_conntrack_core.c           | 78 ++++++++++++++++++++++++++---
 net/netfilter/nf_conntrack_pptp.c           | 62 +++++++++++++----------
 net/netfilter/nfnetlink_cthelper.c          |  3 +-
 7 files changed, 119 insertions(+), 41 deletions(-)

[PATCH 1/5] netfilter: nft_reject_bridge: enable reject with bridge vlan

[Pablo Neira Ayuso]

From: Michael Braun <michael-dev@fami-braun.de>

Currently, using the bridge reject target with tagged packets
results in untagged packets being sent back.

Fix this by mirroring the vlan id as well.

Fixes: 85f5b3086a04 ("netfilter: bridge: add reject support")
Signed-off-by: Michael Braun <michael-dev@fami-braun.de>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/bridge/netfilter/nft_reject_bridge.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/net/bridge/netfilter/nft_reject_bridge.c b/net/bridge/netfilter/nft_reject_bridge.c
index b325b569e761..f48cf4cfb80f 100644
--- a/net/bridge/netfilter/nft_reject_bridge.c
+++ b/net/bridge/netfilter/nft_reject_bridge.c
@@ -31,6 +31,12 @@ static void nft_reject_br_push_etherhdr(struct sk_buff *oldskb,
 	ether_addr_copy(eth->h_dest, eth_hdr(oldskb)->h_source);
 	eth->h_proto = eth_hdr(oldskb)->h_proto;
 	skb_pull(nskb, ETH_HLEN);
+
+	if (skb_vlan_tag_present(oldskb)) {
+		u16 vid = skb_vlan_tag_get(oldskb);
+
+		__vlan_hwaccel_put_tag(nskb, oldskb->vlan_proto, vid);
+	}
 }
 
 static int nft_bridge_iphdr_validate(struct sk_buff *skb)
-- 
2.20.1

[PATCH 2/5] netfilter: ipset: Fix subcounter update skip

[Pablo Neira Ayuso]

From: Phil Sutter <phil@nwl.cc>

If IPSET_FLAG_SKIP_SUBCOUNTER_UPDATE is set, user requested to not
update counters in sub sets. Therefore IPSET_FLAG_SKIP_COUNTER_UPDATE
must be set, not unset.

Fixes: 6e01781d1c80e ("netfilter: ipset: set match: add support to match the counters")
Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/netfilter/ipset/ip_set_list_set.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/netfilter/ipset/ip_set_list_set.c b/net/netfilter/ipset/ip_set_list_set.c
index cd747c0962fd..5a67f7966574 100644
--- a/net/netfilter/ipset/ip_set_list_set.c
+++ b/net/netfilter/ipset/ip_set_list_set.c
@@ -59,7 +59,7 @@ list_set_ktest(struct ip_set *set, const struct sk_buff *skb,
 	/* Don't lookup sub-counters at all */
 	opt->cmdflags &= ~IPSET_FLAG_MATCH_COUNTERS;
 	if (opt->cmdflags & IPSET_FLAG_SKIP_SUBCOUNTER_UPDATE)
-		opt->cmdflags &= ~IPSET_FLAG_SKIP_COUNTER_UPDATE;
+		opt->cmdflags |= IPSET_FLAG_SKIP_COUNTER_UPDATE;
 	list_for_each_entry_rcu(e, &map->members, list) {
 		ret = ip_set_test(e->id, skb, par, opt);
 		if (ret <= 0)
-- 
2.20.1

[PATCH 3/5] netfilter: nf_conntrack_pptp: prevent buffer overflows in debug code

[Pablo Neira Ayuso]

Dan Carpenter says: "Smatch complains that the value for "cmd" comes
from the network and can't be trusted."

Add pptp_msg_name() helper function that checks for the array boundary.

Fixes: f09943fefe6b ("[NETFILTER]: nf_conntrack/nf_nat: add PPTP helper port")
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 include/linux/netfilter/nf_conntrack_pptp.h |  2 +-
 net/ipv4/netfilter/nf_nat_pptp.c            |  7 +--
 net/netfilter/nf_conntrack_pptp.c           | 62 ++++++++++++---------
 3 files changed, 38 insertions(+), 33 deletions(-)

diff --git a/include/linux/netfilter/nf_conntrack_pptp.h b/include/linux/netfilter/nf_conntrack_pptp.h
index fcc409de31a4..6a4ff6d5ebc2 100644
--- a/include/linux/netfilter/nf_conntrack_pptp.h
+++ b/include/linux/netfilter/nf_conntrack_pptp.h
@@ -10,7 +10,7 @@
 #include <net/netfilter/nf_conntrack_expect.h>
 #include <uapi/linux/netfilter/nf_conntrack_tuple_common.h>
 
-extern const char *const pptp_msg_name[];
+extern const char *const pptp_msg_name(u_int16_t msg);
 
 /* state of the control session */
 enum pptp_ctrlsess_state {
diff --git a/net/ipv4/netfilter/nf_nat_pptp.c b/net/ipv4/netfilter/nf_nat_pptp.c
index 3c25a467b3ef..7afde8828b4c 100644
--- a/net/ipv4/netfilter/nf_nat_pptp.c
+++ b/net/ipv4/netfilter/nf_nat_pptp.c
@@ -166,8 +166,7 @@ pptp_outbound_pkt(struct sk_buff *skb,
 		break;
 	default:
 		pr_debug("unknown outbound packet 0x%04x:%s\n", msg,
-			 msg <= PPTP_MSG_MAX ? pptp_msg_name[msg] :
-					       pptp_msg_name[0]);
+			 pptp_msg_name(msg));
 		fallthrough;
 	case PPTP_SET_LINK_INFO:
 		/* only need to NAT in case PAC is behind NAT box */
@@ -268,9 +267,7 @@ pptp_inbound_pkt(struct sk_buff *skb,
 		pcid_off = offsetof(union pptp_ctrl_union, setlink.peersCallID);
 		break;
 	default:
-		pr_debug("unknown inbound packet %s\n",
-			 msg <= PPTP_MSG_MAX ? pptp_msg_name[msg] :
-					       pptp_msg_name[0]);
+		pr_debug("unknown inbound packet %s\n", pptp_msg_name(msg));
 		fallthrough;
 	case PPTP_START_SESSION_REQUEST:
 	case PPTP_START_SESSION_REPLY:
diff --git a/net/netfilter/nf_conntrack_pptp.c b/net/netfilter/nf_conntrack_pptp.c
index a971183f11af..7ad247784cfa 100644
--- a/net/netfilter/nf_conntrack_pptp.c
+++ b/net/netfilter/nf_conntrack_pptp.c
@@ -72,24 +72,32 @@ EXPORT_SYMBOL_GPL(nf_nat_pptp_hook_expectfn);
 
 #if defined(DEBUG) || defined(CONFIG_DYNAMIC_DEBUG)
 /* PptpControlMessageType names */
-const char *const pptp_msg_name[] = {
-	"UNKNOWN_MESSAGE",
-	"START_SESSION_REQUEST",
-	"START_SESSION_REPLY",
-	"STOP_SESSION_REQUEST",
-	"STOP_SESSION_REPLY",
-	"ECHO_REQUEST",
-	"ECHO_REPLY",
-	"OUT_CALL_REQUEST",
-	"OUT_CALL_REPLY",
-	"IN_CALL_REQUEST",
-	"IN_CALL_REPLY",
-	"IN_CALL_CONNECT",
-	"CALL_CLEAR_REQUEST",
-	"CALL_DISCONNECT_NOTIFY",
-	"WAN_ERROR_NOTIFY",
-	"SET_LINK_INFO"
+static const char *const pptp_msg_name_array[PPTP_MSG_MAX + 1] = {
+	[0]				= "UNKNOWN_MESSAGE",
+	[PPTP_START_SESSION_REQUEST]	= "START_SESSION_REQUEST",
+	[PPTP_START_SESSION_REPLY]	= "START_SESSION_REPLY",
+	[PPTP_STOP_SESSION_REQUEST]	= "STOP_SESSION_REQUEST",
+	[PPTP_STOP_SESSION_REPLY]	= "STOP_SESSION_REPLY",
+	[PPTP_ECHO_REQUEST]		= "ECHO_REQUEST",
+	[PPTP_ECHO_REPLY]		= "ECHO_REPLY",
+	[PPTP_OUT_CALL_REQUEST]		= "OUT_CALL_REQUEST",
+	[PPTP_OUT_CALL_REPLY]		= "OUT_CALL_REPLY",
+	[PPTP_IN_CALL_REQUEST]		= "IN_CALL_REQUEST",
+	[PPTP_IN_CALL_REPLY]		= "IN_CALL_REPLY",
+	[PPTP_IN_CALL_CONNECT]		= "IN_CALL_CONNECT",
+	[PPTP_CALL_CLEAR_REQUEST]	= "CALL_CLEAR_REQUEST",
+	[PPTP_CALL_DISCONNECT_NOTIFY]	= "CALL_DISCONNECT_NOTIFY",
+	[PPTP_WAN_ERROR_NOTIFY]		= "WAN_ERROR_NOTIFY",
+	[PPTP_SET_LINK_INFO]		= "SET_LINK_INFO"
 };
+
+const char *const pptp_msg_name(u_int16_t msg)
+{
+	if (msg > PPTP_MSG_MAX)
+		return pptp_msg_name_array[0];
+
+	return pptp_msg_name_array[msg];
+}
 EXPORT_SYMBOL(pptp_msg_name);
 #endif
 
@@ -276,7 +284,7 @@ pptp_inbound_pkt(struct sk_buff *skb, unsigned int protoff,
 	typeof(nf_nat_pptp_hook_inbound) nf_nat_pptp_inbound;
 
 	msg = ntohs(ctlh->messageType);
-	pr_debug("inbound control message %s\n", pptp_msg_name[msg]);
+	pr_debug("inbound control message %s\n", pptp_msg_name(msg));
 
 	switch (msg) {
 	case PPTP_START_SESSION_REPLY:
@@ -311,7 +319,7 @@ pptp_inbound_pkt(struct sk_buff *skb, unsigned int protoff,
 		pcid = pptpReq->ocack.peersCallID;
 		if (info->pns_call_id != pcid)
 			goto invalid;
-		pr_debug("%s, CID=%X, PCID=%X\n", pptp_msg_name[msg],
+		pr_debug("%s, CID=%X, PCID=%X\n", pptp_msg_name(msg),
 			 ntohs(cid), ntohs(pcid));
 
 		if (pptpReq->ocack.resultCode == PPTP_OUTCALL_CONNECT) {
@@ -328,7 +336,7 @@ pptp_inbound_pkt(struct sk_buff *skb, unsigned int protoff,
 			goto invalid;
 
 		cid = pptpReq->icreq.callID;
-		pr_debug("%s, CID=%X\n", pptp_msg_name[msg], ntohs(cid));
+		pr_debug("%s, CID=%X\n", pptp_msg_name(msg), ntohs(cid));
 		info->cstate = PPTP_CALL_IN_REQ;
 		info->pac_call_id = cid;
 		break;
@@ -347,7 +355,7 @@ pptp_inbound_pkt(struct sk_buff *skb, unsigned int protoff,
 		if (info->pns_call_id != pcid)
 			goto invalid;
 
-		pr_debug("%s, PCID=%X\n", pptp_msg_name[msg], ntohs(pcid));
+		pr_debug("%s, PCID=%X\n", pptp_msg_name(msg), ntohs(pcid));
 		info->cstate = PPTP_CALL_IN_CONF;
 
 		/* we expect a GRE connection from PAC to PNS */
@@ -357,7 +365,7 @@ pptp_inbound_pkt(struct sk_buff *skb, unsigned int protoff,
 	case PPTP_CALL_DISCONNECT_NOTIFY:
 		/* server confirms disconnect */
 		cid = pptpReq->disc.callID;
-		pr_debug("%s, CID=%X\n", pptp_msg_name[msg], ntohs(cid));
+		pr_debug("%s, CID=%X\n", pptp_msg_name(msg), ntohs(cid));
 		info->cstate = PPTP_CALL_NONE;
 
 		/* untrack this call id, unexpect GRE packets */
@@ -384,7 +392,7 @@ pptp_inbound_pkt(struct sk_buff *skb, unsigned int protoff,
 invalid:
 	pr_debug("invalid %s: type=%d cid=%u pcid=%u "
 		 "cstate=%d sstate=%d pns_cid=%u pac_cid=%u\n",
-		 msg <= PPTP_MSG_MAX ? pptp_msg_name[msg] : pptp_msg_name[0],
+		 pptp_msg_name(msg),
 		 msg, ntohs(cid), ntohs(pcid),  info->cstate, info->sstate,
 		 ntohs(info->pns_call_id), ntohs(info->pac_call_id));
 	return NF_ACCEPT;
@@ -404,7 +412,7 @@ pptp_outbound_pkt(struct sk_buff *skb, unsigned int protoff,
 	typeof(nf_nat_pptp_hook_outbound) nf_nat_pptp_outbound;
 
 	msg = ntohs(ctlh->messageType);
-	pr_debug("outbound control message %s\n", pptp_msg_name[msg]);
+	pr_debug("outbound control message %s\n", pptp_msg_name(msg));
 
 	switch (msg) {
 	case PPTP_START_SESSION_REQUEST:
@@ -426,7 +434,7 @@ pptp_outbound_pkt(struct sk_buff *skb, unsigned int protoff,
 		info->cstate = PPTP_CALL_OUT_REQ;
 		/* track PNS call id */
 		cid = pptpReq->ocreq.callID;
-		pr_debug("%s, CID=%X\n", pptp_msg_name[msg], ntohs(cid));
+		pr_debug("%s, CID=%X\n", pptp_msg_name(msg), ntohs(cid));
 		info->pns_call_id = cid;
 		break;
 
@@ -440,7 +448,7 @@ pptp_outbound_pkt(struct sk_buff *skb, unsigned int protoff,
 		pcid = pptpReq->icack.peersCallID;
 		if (info->pac_call_id != pcid)
 			goto invalid;
-		pr_debug("%s, CID=%X PCID=%X\n", pptp_msg_name[msg],
+		pr_debug("%s, CID=%X PCID=%X\n", pptp_msg_name(msg),
 			 ntohs(cid), ntohs(pcid));
 
 		if (pptpReq->icack.resultCode == PPTP_INCALL_ACCEPT) {
@@ -480,7 +488,7 @@ pptp_outbound_pkt(struct sk_buff *skb, unsigned int protoff,
 invalid:
 	pr_debug("invalid %s: type=%d cid=%u pcid=%u "
 		 "cstate=%d sstate=%d pns_cid=%u pac_cid=%u\n",
-		 msg <= PPTP_MSG_MAX ? pptp_msg_name[msg] : pptp_msg_name[0],
+		 pptp_msg_name(msg),
 		 msg, ntohs(cid), ntohs(pcid),  info->cstate, info->sstate,
 		 ntohs(info->pns_call_id), ntohs(info->pac_call_id));
 	return NF_ACCEPT;
-- 
2.20.1

[PATCH 4/5] netfilter: conntrack: make conntrack userspace helpers work again

[Pablo Neira Ayuso]

Florian Westphal says:

"Problem is that after the helper hook was merged back into the confirm
one, the queueing itself occurs from the confirm hook, i.e. we queue
from the last netfilter callback in the hook-list.

Therefore, on return, the packet bypasses the confirm action and the
connection is never committed to the main conntrack table.

To fix this there are several ways:
1. revert the 'Fixes' commit and have a extra helper hook again.
   Works, but has the drawback of adding another indirect call for
   everyone.

2. Special case this: split the hooks only when userspace helper
   gets added, so queueing occurs at a lower priority again,
   and normal enqueue reinject would eventually call the last hook.

3. Extend the existing nf_queue ct update hook to allow a forced
   confirmation (plus run the seqadj code).

This goes for 3)."

Fixes: 827318feb69cb ("netfilter: conntrack: remove helper hook again")
Reviewed-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/netfilter/nf_conntrack_core.c | 78 ++++++++++++++++++++++++++++---
 1 file changed, 72 insertions(+), 6 deletions(-)

diff --git a/net/netfilter/nf_conntrack_core.c b/net/netfilter/nf_conntrack_core.c
index 1d57b95d3481..08e0c19f6b39 100644
--- a/net/netfilter/nf_conntrack_core.c
+++ b/net/netfilter/nf_conntrack_core.c
@@ -2016,22 +2016,18 @@ static void nf_conntrack_attach(struct sk_buff *nskb, const struct sk_buff *skb)
 	nf_conntrack_get(skb_nfct(nskb));
 }
 
-static int nf_conntrack_update(struct net *net, struct sk_buff *skb)
+static int __nf_conntrack_update(struct net *net, struct sk_buff *skb,
+				 struct nf_conn *ct)
 {
 	struct nf_conntrack_tuple_hash *h;
 	struct nf_conntrack_tuple tuple;
 	enum ip_conntrack_info ctinfo;
 	struct nf_nat_hook *nat_hook;
 	unsigned int status;
-	struct nf_conn *ct;
 	int dataoff;
 	u16 l3num;
 	u8 l4num;
 
-	ct = nf_ct_get(skb, &ctinfo);
-	if (!ct || nf_ct_is_confirmed(ct))
-		return 0;
-
 	l3num = nf_ct_l3num(ct);
 
 	dataoff = get_l4proto(skb, skb_network_offset(skb), l3num, &l4num);
@@ -2088,6 +2084,76 @@ static int nf_conntrack_update(struct net *net, struct sk_buff *skb)
 	return 0;
 }
 
+/* This packet is coming from userspace via nf_queue, complete the packet
+ * processing after the helper invocation in nf_confirm().
+ */
+static int nf_confirm_cthelper(struct sk_buff *skb, struct nf_conn *ct,
+			       enum ip_conntrack_info ctinfo)
+{
+	const struct nf_conntrack_helper *helper;
+	const struct nf_conn_help *help;
+	unsigned int protoff;
+
+	help = nfct_help(ct);
+	if (!help)
+		return 0;
+
+	helper = rcu_dereference(help->helper);
+	if (!(helper->flags & NF_CT_HELPER_F_USERSPACE))
+		return 0;
+
+	switch (nf_ct_l3num(ct)) {
+	case NFPROTO_IPV4:
+		protoff = skb_network_offset(skb) + ip_hdrlen(skb);
+		break;
+#if IS_ENABLED(CONFIG_IPV6)
+	case NFPROTO_IPV6: {
+		__be16 frag_off;
+		u8 pnum;
+
+		pnum = ipv6_hdr(skb)->nexthdr;
+		protoff = ipv6_skip_exthdr(skb, sizeof(struct ipv6hdr), &pnum,
+					   &frag_off);
+		if (protoff < 0 || (frag_off & htons(~0x7)) != 0)
+			return 0;
+		break;
+	}
+#endif
+	default:
+		return 0;
+	}
+
+	if (test_bit(IPS_SEQ_ADJUST_BIT, &ct->status) &&
+	    !nf_is_loopback_packet(skb)) {
+		if (!nf_ct_seq_adjust(skb, ct, ctinfo, protoff)) {
+			NF_CT_STAT_INC_ATOMIC(nf_ct_net(ct), drop);
+			return -1;
+		}
+	}
+
+	/* We've seen it coming out the other side: confirm it */
+	return nf_conntrack_confirm(skb) == NF_DROP ? - 1 : 0;
+}
+
+static int nf_conntrack_update(struct net *net, struct sk_buff *skb)
+{
+	enum ip_conntrack_info ctinfo;
+	struct nf_conn *ct;
+	int err;
+
+	ct = nf_ct_get(skb, &ctinfo);
+	if (!ct)
+		return 0;
+
+	if (!nf_ct_is_confirmed(ct)) {
+		err = __nf_conntrack_update(net, skb, ct);
+		if (err < 0)
+			return err;
+	}
+
+	return nf_confirm_cthelper(skb, ct, ctinfo);
+}
+
 static bool nf_conntrack_get_tuple_skb(struct nf_conntrack_tuple *dst_tuple,
 				       const struct sk_buff *skb)
 {
-- 
2.20.1

[PATCH 5/5] netfilter: nfnetlink_cthelper: unbreak userspace helper support

[Pablo Neira Ayuso]

Restore helper data size initialization and fix memcopy of the helper
data size.

Fixes: 157ffffeb5dc ("netfilter: nfnetlink_cthelper: reject too large userspace allocation requests")
Reviewed-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/netfilter/nfnetlink_cthelper.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/netfilter/nfnetlink_cthelper.c b/net/netfilter/nfnetlink_cthelper.c
index a5f294aa8e4c..5b0d0a77379c 100644
--- a/net/netfilter/nfnetlink_cthelper.c
+++ b/net/netfilter/nfnetlink_cthelper.c
@@ -103,7 +103,7 @@ nfnl_cthelper_from_nlattr(struct nlattr *attr, struct nf_conn *ct)
 	if (help->helper->data_len == 0)
 		return -EINVAL;
 
-	nla_memcpy(help->data, nla_data(attr), sizeof(help->data));
+	nla_memcpy(help->data, attr, sizeof(help->data));
 	return 0;
 }
 
@@ -240,6 +240,7 @@ nfnl_cthelper_create(const struct nlattr * const tb[],
 		ret = -ENOMEM;
 		goto err2;
 	}
+	helper->data_len = size;
 
 	helper->flags |= NF_CT_HELPER_F_USERSPACE;
 	memcpy(&helper->tuple, tuple, sizeof(struct nf_conntrack_tuple));
-- 
2.20.1

Re: [PATCH 0/5] Netfilter fixes for net

[David Miller]

From: Pablo Neira Ayuso <pablo@netfilter.org>
Date: Mon, 25 May 2020 23:54:15 +0200

> The following patchset contains Netfilter fixes for net:
> 
> 1) Set VLAN tag in tcp reset/icmp unreachable packets to reject
>    connections in the bridge family, from Michael Braun.
> 
> 2) Incorrect subcounter flag update in ipset, from Phil Sutter.
> 
> 3) Possible buffer overflow in the pptp conntrack helper, based
>    on patch from Dan Carpenter.
> 
> 4) Restore userspace conntrack helper hook logic that broke after
>    hook consolidation rework.
> 
> 5) Unbreak userspace conntrack helper registration via
>    nfnetlink_cthelper.
> 
> You can pull these changes from:
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/pablo/nf.git

Pulled, thank you.

Re: [PATCH 4/5] netfilter: conntrack: make conntrack userspace helpers work again

[Jakub Kicinski]

On Mon, 25 May 2020 23:54:19 +0200 Pablo Neira Ayuso wrote:
> +/* This packet is coming from userspace via nf_queue, complete the packet
> + * processing after the helper invocation in nf_confirm().
> + */
> +static int nf_confirm_cthelper(struct sk_buff *skb, struct nf_conn *ct,
> +			       enum ip_conntrack_info ctinfo)
> +{
> +	const struct nf_conntrack_helper *helper;
> +	const struct nf_conn_help *help;
> +	unsigned int protoff;
> +
> +	help = nfct_help(ct);
> +	if (!help)
> +		return 0;
> +
> +	helper = rcu_dereference(help->helper);
> +	if (!(helper->flags & NF_CT_HELPER_F_USERSPACE))
> +		return 0;
> +
> +	switch (nf_ct_l3num(ct)) {
> +	case NFPROTO_IPV4:
> +		protoff = skb_network_offset(skb) + ip_hdrlen(skb);
> +		break;
> +#if IS_ENABLED(CONFIG_IPV6)
> +	case NFPROTO_IPV6: {
> +		__be16 frag_off;
> +		u8 pnum;
> +
> +		pnum = ipv6_hdr(skb)->nexthdr;
> +		protoff = ipv6_skip_exthdr(skb, sizeof(struct ipv6hdr), &pnum,
> +					   &frag_off);
> +		if (protoff < 0 || (frag_off & htons(~0x7)) != 0)
> +			return 0;
> +		break;
> +	}

net/netfilter/nf_conntrack_core.c: In function nf_confirm_cthelper:
net/netfilter/nf_conntrack_core.c:2117:15: warning: comparison of unsigned expression in < 0 is always false [-Wtype-limits]
 2117 |   if (protoff < 0 || (frag_off & htons(~0x7)) != 0)
      |               ^

Re: [PATCH 0/5] Netfilter fixes for net

[Pablo Neira Ayuso]

On Mon, May 25, 2020 at 06:29:01PM -0700, David Miller wrote:
> From: Pablo Neira Ayuso <pablo@netfilter.org>
> Date: Mon, 25 May 2020 23:54:15 +0200
> 
> > The following patchset contains Netfilter fixes for net:
> > 
> > 1) Set VLAN tag in tcp reset/icmp unreachable packets to reject
> >    connections in the bridge family, from Michael Braun.
> > 
> > 2) Incorrect subcounter flag update in ipset, from Phil Sutter.
> > 
> > 3) Possible buffer overflow in the pptp conntrack helper, based
> >    on patch from Dan Carpenter.
> > 
> > 4) Restore userspace conntrack helper hook logic that broke after
> >    hook consolidation rework.
> > 
> > 5) Unbreak userspace conntrack helper registration via
> >    nfnetlink_cthelper.
> > 
> > You can pull these changes from:
> > 
> >   git://git.kernel.org/pub/scm/linux/kernel/git/pablo/nf.git
> 
> Pulled, thank you.

If it's still possible, it would be good to toss this pull request.

Otherwise, I will send another pull request to address the kbuild
reports.

Thank you.

Re: [PATCH 0/5] Netfilter fixes for net

[David Miller]

From: Pablo Neira Ayuso <pablo@netfilter.org>
Date: Tue, 26 May 2020 22:10:23 +0200

> If it's still possible, it would be good to toss this pull request.
> 
> Otherwise, I will send another pull request to address the kbuild
> reports.

Unfortunately I pushed it out already, please send me follow-ups.

Thanks.

Re: [PATCH 3/5] netfilter: nf_conntrack_pptp: prevent buffer overflows in debug code

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 2856 bytes --]

Hi Pablo,

I love your patch! Perhaps something to improve:

[auto build test WARNING on nf-next/master]
[also build test WARNING on net-next/master v5.7-rc7]
[cannot apply to nf/master net/master next-20200526]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Pablo-Neira-Ayuso/Netfilter-fixes-for-net/20200526-055708
base:   https://git.kernel.org/pub/scm/linux/kernel/git/pablo/nf-next.git master
config: alpha-randconfig-r024-20200527 (attached as .config)
compiler: alpha-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=alpha 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kbuild test robot <lkp@intel.com>

All warnings (new ones prefixed by >>, old ones prefixed by <<):

In file included from include/net/netfilter/nf_nat.h:7,
from include/net/tc_act/tc_ct.h:9,
from net/sched/cls_api.c:40:
>> include/linux/netfilter/nf_conntrack_pptp.h:13:8: warning: type qualifiers ignored on function return type [-Wignored-qualifiers]
13 | extern const char *const pptp_msg_name(u_int16_t msg);
|        ^~~~~
--
In file included from net/netfilter/nf_conntrack_pptp.c:36:
>> include/linux/netfilter/nf_conntrack_pptp.h:13:8: warning: type qualifiers ignored on function return type [-Wignored-qualifiers]
13 | extern const char *const pptp_msg_name(u_int16_t msg);
|        ^~~~~
>> net/netfilter/nf_conntrack_pptp.c:94:1: warning: type qualifiers ignored on function return type [-Wignored-qualifiers]
94 | const char *const pptp_msg_name(u_int16_t msg)
| ^~~~~
--
In file included from include/net/netfilter/nf_nat.h:7,
from net/ipv4/netfilter/nf_nat_snmp_basic_main.c:43:
>> include/linux/netfilter/nf_conntrack_pptp.h:13:8: warning: type qualifiers ignored on function return type [-Wignored-qualifiers]
13 | extern const char *const pptp_msg_name(u_int16_t msg);
|        ^~~~~
net/ipv4/netfilter/nf_nat_snmp_basic_main.c:47:10: fatal error: nf_nat_snmp_basic.asn1.h: No such file or directory
47 | #include "nf_nat_snmp_basic.asn1.h"
|          ^~~~~~~~~~~~~~~~~~~~~~~~~~
compilation terminated.

vim +13 include/linux/netfilter/nf_conntrack_pptp.h

    12	
  > 13	extern const char *const pptp_msg_name(u_int16_t msg);
    14	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 33108 bytes --]

Re: [PATCH 4/5] netfilter: conntrack: make conntrack userspace helpers work again

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 12181 bytes --]

Hi Pablo,

I love your patch! Perhaps something to improve:

[auto build test WARNING on nf-next/master]
[also build test WARNING on net-next/master v5.7-rc7]
[cannot apply to nf/master net/master next-20200526]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Pablo-Neira-Ayuso/Netfilter-fixes-for-net/20200526-055708
base:   https://git.kernel.org/pub/scm/linux/kernel/git/pablo/nf-next.git master
config: mips-randconfig-r026-20200527 (attached as .config)
compiler: mipsel-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=mips 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kbuild test robot <lkp@intel.com>

All warnings (new ones prefixed by >>, old ones prefixed by <<):

In file included from include/net/netfilter/nf_nat.h:7,
from net/netfilter/nf_conntrack_core.c:51:
include/linux/netfilter/nf_conntrack_pptp.h:13:8: warning: type qualifiers ignored on function return type [-Wignored-qualifiers]
13 | extern const char *const pptp_msg_name(u_int16_t msg);
|        ^~~~~
In file included from include/linux/init.h:5,
from include/linux/netfilter.h:5,
from net/netfilter/nf_conntrack_core.c:15:
net/netfilter/nf_conntrack_core.c: In function 'nf_confirm_cthelper':
net/netfilter/nf_conntrack_core.c:2117:15: warning: comparison of unsigned expression < 0 is always false [-Wtype-limits]
2117 |   if (protoff < 0 || (frag_off & htons(~0x7)) != 0)
|               ^
include/linux/compiler.h:58:52: note: in definition of macro '__trace_if_var'
58 | #define __trace_if_var(cond) (__builtin_constant_p(cond) ? (cond) : __trace_if_value(cond))
|                                                    ^~~~
>> net/netfilter/nf_conntrack_core.c:2117:3: note: in expansion of macro 'if'
2117 |   if (protoff < 0 || (frag_off & htons(~0x7)) != 0)
|   ^~
net/netfilter/nf_conntrack_core.c:2117:15: warning: comparison of unsigned expression < 0 is always false [-Wtype-limits]
2117 |   if (protoff < 0 || (frag_off & htons(~0x7)) != 0)
|               ^
include/linux/compiler.h:58:61: note: in definition of macro '__trace_if_var'
58 | #define __trace_if_var(cond) (__builtin_constant_p(cond) ? (cond) : __trace_if_value(cond))
|                                                             ^~~~
>> net/netfilter/nf_conntrack_core.c:2117:3: note: in expansion of macro 'if'
2117 |   if (protoff < 0 || (frag_off & htons(~0x7)) != 0)
|   ^~
net/netfilter/nf_conntrack_core.c:2117:15: warning: comparison of unsigned expression < 0 is always false [-Wtype-limits]
2117 |   if (protoff < 0 || (frag_off & htons(~0x7)) != 0)
|               ^
include/linux/compiler.h:69:3: note: in definition of macro '__trace_if_value'
69 |  (cond) ?              |   ^~~~
include/linux/compiler.h:56:28: note: in expansion of macro '__trace_if_var'
56 | #define if(cond, ...) if ( __trace_if_var( !!(cond , ## __VA_ARGS__) ) )
|                            ^~~~~~~~~~~~~~
>> net/netfilter/nf_conntrack_core.c:2117:3: note: in expansion of macro 'if'
2117 |   if (protoff < 0 || (frag_off & htons(~0x7)) != 0)
|   ^~
{standard input}: Assembler messages:
{standard input}:4679: Error: found '(', expected: ')'
{standard input}:4679: Error: found '(', expected: ')'
{standard input}:4679: Error: non-constant expression in ".if" statement
{standard input}:4679: Error: junk at end of line, first unrecognized character is `('
{standard input}:4688: Error: found '(', expected: ')'
{standard input}:4688: Error: found '(', expected: ')'
{standard input}:4688: Error: non-constant expression in ".if" statement
{standard input}:4688: Error: junk at end of line, first unrecognized character is `('
{standard input}:4918: Error: found '(', expected: ')'
{standard input}:4918: Error: found '(', expected: ')'
{standard input}:4918: Error: non-constant expression in ".if" statement
{standard input}:4918: Error: junk at end of line, first unrecognized character is `('
{standard input}:5004: Error: found '(', expected: ')'
{standard input}:5004: Error: found '(', expected: ')'
{standard input}:5004: Error: non-constant expression in ".if" statement
{standard input}:5004: Error: junk at end of line, first unrecognized character is `('
{standard input}:5177: Error: found '(', expected: ')'
{standard input}:5177: Error: found '(', expected: ')'
{standard input}:5177: Error: non-constant expression in ".if" statement
{standard input}:5177: Error: junk at end of line, first unrecognized character is `('
{standard input}:5868: Error: found '(', expected: ')'
{standard input}:5868: Error: found '(', expected: ')'
{standard input}:5868: Error: non-constant expression in ".if" statement
{standard input}:5868: Error: junk at end of line, first unrecognized character is `('
{standard input}:6904: Error: found '(', expected: ')'
{standard input}:6904: Error: found '(', expected: ')'
{standard input}:6904: Error: non-constant expression in ".if" statement
{standard input}:6904: Error: junk at end of line, first unrecognized character is `('
{standard input}:9093: Error: found '(', expected: ')'
{standard input}:9093: Error: found '(', expected: ')'
{standard input}:9093: Error: non-constant expression in ".if" statement
{standard input}:9093: Error: junk at end of line, first unrecognized character is `('
{standard input}:14265: Error: found '(', expected: ')'
{standard input}:14265: Error: found '(', expected: ')'
{standard input}:14265: Error: non-constant expression in ".if" statement
{standard input}:14265: Error: junk at end of line, first unrecognized character is `('
{standard input}:15722: Error: found '(', expected: ')'
{standard input}:15722: Error: found '(', expected: ')'
{standard input}:15722: Error: non-constant expression in ".if" statement
{standard input}:15722: Error: junk at end of line, first unrecognized character is `('
{standard input}:17070: Error: found '(', expected: ')'
{standard input}:17070: Error: found '(', expected: ')'
{standard input}:17070: Error: non-constant expression in ".if" statement
{standard input}:17070: Error: junk at end of line, first unrecognized character is `('
{standard input}:17449: Error: found '(', expected: ')'
{standard input}:17449: Error: found '(', expected: ')'
{standard input}:17449: Error: non-constant expression in ".if" statement
{standard input}:17449: Error: junk at end of line, first unrecognized character is `('
{standard input}:17458: Error: found '(', expected: ')'
{standard input}:17458: Error: found '(', expected: ')'
{standard input}:17458: Error: non-constant expression in ".if" statement
{standard input}:17458: Error: junk at end of line, first unrecognized character is `('
{standard input}:17941: Error: found '(', expected: ')'
{standard input}:17941: Error: found '(', expected: ')'
{standard input}:17941: Error: non-constant expression in ".if" statement
{standard input}:17941: Error: junk at end of line, first unrecognized character is `('
{standard input}:21320: Error: found '(', expected: ')'
{standard input}:21320: Error: found '(', expected: ')'
{standard input}:21320: Error: non-constant expression in ".if" statement
{standard input}:21320: Error: junk at end of line, first unrecognized character is `('
{standard input}:21329: Error: found '(', expected: ')'
{standard input}:21329: Error: found '(', expected: ')'
{standard input}:21329: Error: non-constant expression in ".if" statement
{standard input}:21329: Error: junk at end of line, first unrecognized character is `('
{standard input}:21776: Error: found '(', expected: ')'
{standard input}:21776: Error: found '(', expected: ')'
{standard input}:21776: Error: non-constant expression in ".if" statement
{standard input}:21776: Error: junk at end of line, first unrecognized character is `('
{standard input}:22309: Error: found '(', expected: ')'
{standard input}:22309: Error: found '(', expected: ')'
{standard input}:22309: Error: non-constant expression in ".if" statement
{standard input}:22309: Error: junk at end of line, first unrecognized character is `('
{standard input}:22529: Error: found '(', expected: ')'
{standard input}:22529: Error: found '(', expected: ')'
{standard input}:22529: Error: non-constant expression in ".if" statement
{standard input}:22529: Error: junk at end of line, first unrecognized character is `('
{standard input}:22647: Error: found '(', expected: ')'
{standard input}:22647: Error: found '(', expected: ')'
{standard input}:22647: Error: non-constant expression in ".if" statement
{standard input}:22647: Error: junk at end of line, first unrecognized character is `('
{standard input}:24358: Error: found '(', expected: ')'
{standard input}:24358: Error: found '(', expected: ')'
{standard input}:24358: Error: non-constant expression in ".if" statement
{standard input}:24358: Error: junk at end of line, first unrecognized character is `('
{standard input}:25621: Error: found '(', expected: ')'
{standard input}:25621: Error: found '(', expected: ')'
{standard input}:25621: Error: non-constant expression in ".if" statement
{standard input}:25621: Error: junk at end of line, first unrecognized character is `('
{standard input}:25974: Error: found '(', expected: ')'
{standard input}:25974: Error: found '(', expected: ')'
{standard input}:25974: Error: non-constant expression in ".if" statement
{standard input}:25974: Error: junk at end of line, first unrecognized character is `('
{standard input}:28021: Error: found '(', expected: ')'
{standard input}:28021: Error: found '(', expected: ')'
{standard input}:28021: Error: non-constant expression in ".if" statement
{standard input}:28021: Error: junk@end of line, first unrecognized character is `('
{standard input}:28030: Error: found '(', expected: ')'

vim +/if +2117 net/netfilter/nf_conntrack_core.c

  2086	
  2087	/* This packet is coming from userspace via nf_queue, complete the packet
  2088	 * processing after the helper invocation in nf_confirm().
  2089	 */
  2090	static int nf_confirm_cthelper(struct sk_buff *skb, struct nf_conn *ct,
  2091				       enum ip_conntrack_info ctinfo)
  2092	{
  2093		const struct nf_conntrack_helper *helper;
  2094		const struct nf_conn_help *help;
  2095		unsigned int protoff;
  2096	
  2097		help = nfct_help(ct);
  2098		if (!help)
  2099			return 0;
  2100	
  2101		helper = rcu_dereference(help->helper);
  2102		if (!(helper->flags & NF_CT_HELPER_F_USERSPACE))
  2103			return 0;
  2104	
  2105		switch (nf_ct_l3num(ct)) {
  2106		case NFPROTO_IPV4:
  2107			protoff = skb_network_offset(skb) + ip_hdrlen(skb);
  2108			break;
  2109	#if IS_ENABLED(CONFIG_IPV6)
  2110		case NFPROTO_IPV6: {
  2111			__be16 frag_off;
  2112			u8 pnum;
  2113	
  2114			pnum = ipv6_hdr(skb)->nexthdr;
  2115			protoff = ipv6_skip_exthdr(skb, sizeof(struct ipv6hdr), &pnum,
  2116						   &frag_off);
> 2117			if (protoff < 0 || (frag_off & htons(~0x7)) != 0)
  2118				return 0;
  2119			break;
  2120		}
  2121	#endif
  2122		default:
  2123			return 0;
  2124		}
  2125	
  2126		if (test_bit(IPS_SEQ_ADJUST_BIT, &ct->status) &&
  2127		    !nf_is_loopback_packet(skb)) {
  2128			if (!nf_ct_seq_adjust(skb, ct, ctinfo, protoff)) {
  2129				NF_CT_STAT_INC_ATOMIC(nf_ct_net(ct), drop);
  2130				return -1;
  2131			}
  2132		}
  2133	
  2134		/* We've seen it coming out the other side: confirm it */
  2135		return nf_conntrack_confirm(skb) == NF_DROP ? - 1 : 0;
  2136	}
  2137	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 32338 bytes --]