Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH v6 18/18] crypto: Remove AHASH_REQUEST_ON_STACK
From: Kees Cook @ 2018-07-24 17:53 UTC (permalink / raw)
  To: Joe Perches
  Cc: Herbert Xu, Arnd Bergmann, Eric Biggers, Gustavo A. R. Silva,
	Alasdair Kergon, Rabin Vincent, Tim Chen, Rafael J. Wysocki,
	Pavel Machek, Thomas Gleixner, Ingo Molnar, H. Peter Anvin,
	X86 ML, Philipp Reisner, Lars Ellenberg, Jens Axboe,
	Giovanni Cabiddu, Mike Snitzer, Paul Mackerras,
	Greg Kroah-Hartman
In-Reply-To: <1bdc706ae86039c4ffcff39698251424d54af116.camel-6d6DIl74uiNBDgjK7y7TUQ@public.gmane.org>

On Tue, Jul 24, 2018 at 10:31 AM, Joe Perches <joe-6d6DIl74uiNBDgjK7y7TUQ@public.gmane.org> wrote:
> On Tue, 2018-07-24 at 09:49 -0700, Kees Cook wrote:
>> All users of AHASH_REQUEST_ON_STACK have been removed from the kernel, so
>> drop it entirely so no VLAs get reintroduced by future users.
>
> checkpatch has a test for that.
> It could now be removed as well.
> ---
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 34e4683de7a3..a3517334d661 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -796,7 +796,7 @@ our $declaration_macros = qr{(?x:
>         (?:$Storage\s+)?(?:[A-Z_][A-Z0-9]*_){0,2}(?:DEFINE|DECLARE)(?:_[A-Z0-9]+){1,6}\s*\(|
>         (?:$Storage\s+)?[HLP]?LIST_HEAD\s*\(|
>         (?:$Storage\s+)?${Type}\s+uninitialized_var\s*\(|
> -       (?:SKCIPHER_REQUEST|SHASH_DESC|AHASH_REQUEST)_ON_STACK\s*\(
> +       (?:SKCIPHER_REQUEST|SHASH_DESC)_ON_STACK\s*\(
>  )};

Ah! Cool. I've added this now.

-Kees

-- 
Kees Cook
Pixel Security

^ permalink raw reply

* Re: [PATCH v6 17/18] crypto: ccm: Remove VLA usage
From: Kees Cook @ 2018-07-24 17:51 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: Herbert Xu, Arnd Bergmann, Eric Biggers, Gustavo A. R. Silva,
	Alasdair Kergon, Rabin Vincent, Tim Chen, Rafael J. Wysocki,
	Pavel Machek, Thomas Gleixner, Ingo Molnar, H. Peter Anvin,
	the arch/x86 maintainers, Philipp Reisner, Lars Ellenberg,
	Jens Axboe, Giovanni Cabiddu, Mike Snitzer, Paul Mackerras,
	Greg Kroah-Hartman <gregk
In-Reply-To: <CAKv+Gu-D9FrOrFU0EmkPA52vrywCGZkFP9vUsyEovdby9dLj0g@mail.gmail.com>

On Tue, Jul 24, 2018 at 9:57 AM, Ard Biesheuvel
<ard.biesheuvel@linaro.org> wrote:
> On 24 July 2018 at 18:49, Kees Cook <keescook@chromium.org> wrote:
>> From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>>
>> In the quest to remove all stack VLA usage from the kernel[1], this
>> drops AHASH_REQUEST_ON_STACK by preallocated the ahash request area
>> with the skcipher area (which are not used at the same time).
>>
>
> -EGRAMMAR

Whoops. Will fix my poor sentence merging. :)

-Kees

-- 
Kees Cook
Pixel Security

^ permalink raw reply

* [PATCH 9/9] netfilter: nf_tables: move dumper state allocation into ->start
From: Pablo Neira Ayuso @ 2018-07-24 16:31 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev
In-Reply-To: <20180724163133.14586-1-pablo@netfilter.org>

From: Florian Westphal <fw@strlen.de>

Shaochun Chen points out we leak dumper filter state allocations
stored in dump_control->data in case there is an error before netlink sets
cb_running (after which ->done will be called at some point).

In order to fix this, add .start functions and do the allocations
there.

->done is going to clean up, and in case error occurs before
->start invocation no cleanups need to be done anymore.

Reported-by: shaochun chen <cscnull@gmail.com>
Signed-off-by: Florian Westphal <fw@strlen.de>
Acked-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/netfilter/nf_tables_api.c | 219 ++++++++++++++++++++++--------------------
 1 file changed, 115 insertions(+), 104 deletions(-)

diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index d7b9748e338e..f5745e4c6513 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -2271,6 +2271,39 @@ static int nf_tables_dump_rules(struct sk_buff *skb,
 	return skb->len;
 }
 
+static int nf_tables_dump_rules_start(struct netlink_callback *cb)
+{
+	const struct nlattr * const *nla = cb->data;
+	struct nft_rule_dump_ctx *ctx = NULL;
+
+	if (nla[NFTA_RULE_TABLE] || nla[NFTA_RULE_CHAIN]) {
+		ctx = kzalloc(sizeof(*ctx), GFP_ATOMIC);
+		if (!ctx)
+			return -ENOMEM;
+
+		if (nla[NFTA_RULE_TABLE]) {
+			ctx->table = nla_strdup(nla[NFTA_RULE_TABLE],
+							GFP_ATOMIC);
+			if (!ctx->table) {
+				kfree(ctx);
+				return -ENOMEM;
+			}
+		}
+		if (nla[NFTA_RULE_CHAIN]) {
+			ctx->chain = nla_strdup(nla[NFTA_RULE_CHAIN],
+						GFP_ATOMIC);
+			if (!ctx->chain) {
+				kfree(ctx->table);
+				kfree(ctx);
+				return -ENOMEM;
+			}
+		}
+	}
+
+	cb->data = ctx;
+	return 0;
+}
+
 static int nf_tables_dump_rules_done(struct netlink_callback *cb)
 {
 	struct nft_rule_dump_ctx *ctx = cb->data;
@@ -2300,38 +2333,13 @@ static int nf_tables_getrule(struct net *net, struct sock *nlsk,
 
 	if (nlh->nlmsg_flags & NLM_F_DUMP) {
 		struct netlink_dump_control c = {
+			.start= nf_tables_dump_rules_start,
 			.dump = nf_tables_dump_rules,
 			.done = nf_tables_dump_rules_done,
 			.module = THIS_MODULE,
+			.data = (void *)nla,
 		};
 
-		if (nla[NFTA_RULE_TABLE] || nla[NFTA_RULE_CHAIN]) {
-			struct nft_rule_dump_ctx *ctx;
-
-			ctx = kzalloc(sizeof(*ctx), GFP_ATOMIC);
-			if (!ctx)
-				return -ENOMEM;
-
-			if (nla[NFTA_RULE_TABLE]) {
-				ctx->table = nla_strdup(nla[NFTA_RULE_TABLE],
-							GFP_ATOMIC);
-				if (!ctx->table) {
-					kfree(ctx);
-					return -ENOMEM;
-				}
-			}
-			if (nla[NFTA_RULE_CHAIN]) {
-				ctx->chain = nla_strdup(nla[NFTA_RULE_CHAIN],
-							GFP_ATOMIC);
-				if (!ctx->chain) {
-					kfree(ctx->table);
-					kfree(ctx);
-					return -ENOMEM;
-				}
-			}
-			c.data = ctx;
-		}
-
 		return nft_netlink_dump_start_rcu(nlsk, skb, nlh, &c);
 	}
 
@@ -3181,6 +3189,18 @@ static int nf_tables_dump_sets(struct sk_buff *skb, struct netlink_callback *cb)
 	return skb->len;
 }
 
+static int nf_tables_dump_sets_start(struct netlink_callback *cb)
+{
+	struct nft_ctx *ctx_dump = NULL;
+
+	ctx_dump = kmemdup(cb->data, sizeof(*ctx_dump), GFP_ATOMIC);
+	if (ctx_dump == NULL)
+		return -ENOMEM;
+
+	cb->data = ctx_dump;
+	return 0;
+}
+
 static int nf_tables_dump_sets_done(struct netlink_callback *cb)
 {
 	kfree(cb->data);
@@ -3208,18 +3228,12 @@ static int nf_tables_getset(struct net *net, struct sock *nlsk,
 
 	if (nlh->nlmsg_flags & NLM_F_DUMP) {
 		struct netlink_dump_control c = {
+			.start = nf_tables_dump_sets_start,
 			.dump = nf_tables_dump_sets,
 			.done = nf_tables_dump_sets_done,
+			.data = &ctx,
 			.module = THIS_MODULE,
 		};
-		struct nft_ctx *ctx_dump;
-
-		ctx_dump = kmalloc(sizeof(*ctx_dump), GFP_ATOMIC);
-		if (ctx_dump == NULL)
-			return -ENOMEM;
-
-		*ctx_dump = ctx;
-		c.data = ctx_dump;
 
 		return nft_netlink_dump_start_rcu(nlsk, skb, nlh, &c);
 	}
@@ -3869,6 +3883,15 @@ static int nf_tables_dump_set(struct sk_buff *skb, struct netlink_callback *cb)
 	return -ENOSPC;
 }
 
+static int nf_tables_dump_set_start(struct netlink_callback *cb)
+{
+	struct nft_set_dump_ctx *dump_ctx = cb->data;
+
+	cb->data = kmemdup(dump_ctx, sizeof(*dump_ctx), GFP_ATOMIC);
+
+	return cb->data ? 0 : -ENOMEM;
+}
+
 static int nf_tables_dump_set_done(struct netlink_callback *cb)
 {
 	kfree(cb->data);
@@ -4022,20 +4045,17 @@ static int nf_tables_getsetelem(struct net *net, struct sock *nlsk,
 
 	if (nlh->nlmsg_flags & NLM_F_DUMP) {
 		struct netlink_dump_control c = {
+			.start = nf_tables_dump_set_start,
 			.dump = nf_tables_dump_set,
 			.done = nf_tables_dump_set_done,
 			.module = THIS_MODULE,
 		};
-		struct nft_set_dump_ctx *dump_ctx;
-
-		dump_ctx = kmalloc(sizeof(*dump_ctx), GFP_ATOMIC);
-		if (!dump_ctx)
-			return -ENOMEM;
-
-		dump_ctx->set = set;
-		dump_ctx->ctx = ctx;
+		struct nft_set_dump_ctx dump_ctx = {
+			.set = set,
+			.ctx = ctx,
+		};
 
-		c.data = dump_ctx;
+		c.data = &dump_ctx;
 		return nft_netlink_dump_start_rcu(nlsk, skb, nlh, &c);
 	}
 
@@ -4995,38 +5015,42 @@ static int nf_tables_dump_obj(struct sk_buff *skb, struct netlink_callback *cb)
 	return skb->len;
 }
 
-static int nf_tables_dump_obj_done(struct netlink_callback *cb)
+static int nf_tables_dump_obj_start(struct netlink_callback *cb)
 {
-	struct nft_obj_filter *filter = cb->data;
+	const struct nlattr * const *nla = cb->data;
+	struct nft_obj_filter *filter = NULL;
 
-	if (filter) {
-		kfree(filter->table);
-		kfree(filter);
+	if (nla[NFTA_OBJ_TABLE] || nla[NFTA_OBJ_TYPE]) {
+		filter = kzalloc(sizeof(*filter), GFP_ATOMIC);
+		if (!filter)
+			return -ENOMEM;
+
+		if (nla[NFTA_OBJ_TABLE]) {
+			filter->table = nla_strdup(nla[NFTA_OBJ_TABLE], GFP_ATOMIC);
+			if (!filter->table) {
+				kfree(filter);
+				return -ENOMEM;
+			}
+		}
+
+		if (nla[NFTA_OBJ_TYPE])
+			filter->type = ntohl(nla_get_be32(nla[NFTA_OBJ_TYPE]));
 	}
 
+	cb->data = filter;
 	return 0;
 }
 
-static struct nft_obj_filter *
-nft_obj_filter_alloc(const struct nlattr * const nla[])
+static int nf_tables_dump_obj_done(struct netlink_callback *cb)
 {
-	struct nft_obj_filter *filter;
-
-	filter = kzalloc(sizeof(*filter), GFP_ATOMIC);
-	if (!filter)
-		return ERR_PTR(-ENOMEM);
+	struct nft_obj_filter *filter = cb->data;
 
-	if (nla[NFTA_OBJ_TABLE]) {
-		filter->table = nla_strdup(nla[NFTA_OBJ_TABLE], GFP_ATOMIC);
-		if (!filter->table) {
-			kfree(filter);
-			return ERR_PTR(-ENOMEM);
-		}
+	if (filter) {
+		kfree(filter->table);
+		kfree(filter);
 	}
-	if (nla[NFTA_OBJ_TYPE])
-		filter->type = ntohl(nla_get_be32(nla[NFTA_OBJ_TYPE]));
 
-	return filter;
+	return 0;
 }
 
 /* called with rcu_read_lock held */
@@ -5047,21 +5071,13 @@ static int nf_tables_getobj(struct net *net, struct sock *nlsk,
 
 	if (nlh->nlmsg_flags & NLM_F_DUMP) {
 		struct netlink_dump_control c = {
+			.start = nf_tables_dump_obj_start,
 			.dump = nf_tables_dump_obj,
 			.done = nf_tables_dump_obj_done,
 			.module = THIS_MODULE,
+			.data = (void *)nla,
 		};
 
-		if (nla[NFTA_OBJ_TABLE] ||
-		    nla[NFTA_OBJ_TYPE]) {
-			struct nft_obj_filter *filter;
-
-			filter = nft_obj_filter_alloc(nla);
-			if (IS_ERR(filter))
-				return -ENOMEM;
-
-			c.data = filter;
-		}
 		return nft_netlink_dump_start_rcu(nlsk, skb, nlh, &c);
 	}
 
@@ -5667,37 +5683,39 @@ static int nf_tables_dump_flowtable(struct sk_buff *skb,
 	return skb->len;
 }
 
-static int nf_tables_dump_flowtable_done(struct netlink_callback *cb)
+static int nf_tables_dump_flowtable_start(struct netlink_callback *cb)
 {
-	struct nft_flowtable_filter *filter = cb->data;
+	const struct nlattr * const *nla = cb->data;
+	struct nft_flowtable_filter *filter = NULL;
 
-	if (!filter)
-		return 0;
+	if (nla[NFTA_FLOWTABLE_TABLE]) {
+		filter = kzalloc(sizeof(*filter), GFP_ATOMIC);
+		if (!filter)
+			return -ENOMEM;
 
-	kfree(filter->table);
-	kfree(filter);
+		filter->table = nla_strdup(nla[NFTA_FLOWTABLE_TABLE],
+					   GFP_ATOMIC);
+		if (!filter->table) {
+			kfree(filter);
+			return -ENOMEM;
+		}
+	}
 
+	cb->data = filter;
 	return 0;
 }
 
-static struct nft_flowtable_filter *
-nft_flowtable_filter_alloc(const struct nlattr * const nla[])
+static int nf_tables_dump_flowtable_done(struct netlink_callback *cb)
 {
-	struct nft_flowtable_filter *filter;
+	struct nft_flowtable_filter *filter = cb->data;
 
-	filter = kzalloc(sizeof(*filter), GFP_ATOMIC);
 	if (!filter)
-		return ERR_PTR(-ENOMEM);
+		return 0;
 
-	if (nla[NFTA_FLOWTABLE_TABLE]) {
-		filter->table = nla_strdup(nla[NFTA_FLOWTABLE_TABLE],
-					   GFP_ATOMIC);
-		if (!filter->table) {
-			kfree(filter);
-			return ERR_PTR(-ENOMEM);
-		}
-	}
-	return filter;
+	kfree(filter->table);
+	kfree(filter);
+
+	return 0;
 }
 
 /* called with rcu_read_lock held */
@@ -5717,20 +5735,13 @@ static int nf_tables_getflowtable(struct net *net, struct sock *nlsk,
 
 	if (nlh->nlmsg_flags & NLM_F_DUMP) {
 		struct netlink_dump_control c = {
+			.start = nf_tables_dump_flowtable_start,
 			.dump = nf_tables_dump_flowtable,
 			.done = nf_tables_dump_flowtable_done,
 			.module = THIS_MODULE,
+			.data = (void *)nla,
 		};
 
-		if (nla[NFTA_FLOWTABLE_TABLE]) {
-			struct nft_flowtable_filter *filter;
-
-			filter = nft_flowtable_filter_alloc(nla);
-			if (IS_ERR(filter))
-				return -ENOMEM;
-
-			c.data = filter;
-		}
 		return nft_netlink_dump_start_rcu(nlsk, skb, nlh, &c);
 	}
 
-- 
2.11.0

^ permalink raw reply related

* [PATCH 4/9] netfilter: nf_tables: use dev->name directly
From: Pablo Neira Ayuso @ 2018-07-24 16:31 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev
In-Reply-To: <20180724163133.14586-1-pablo@netfilter.org>

From: Florian Westphal <fw@strlen.de>

no need to store the name in separate area.

Furthermore, it uses kmalloc but not kfree and most accesses seem to treat
it as char[IFNAMSIZ] not char *.

Remove this and use dev->name instead.

In case event zeroed dev, just omit the name in the dump.

Fixes: d92191aa84e5f1 ("netfilter: nf_tables: cache device name in flowtable object")
Signed-off-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 include/net/netfilter/nf_tables.h |  1 -
 net/netfilter/nf_tables_api.c     | 14 +++++---------
 2 files changed, 5 insertions(+), 10 deletions(-)

diff --git a/include/net/netfilter/nf_tables.h b/include/net/netfilter/nf_tables.h
index 4e82a4c49912..dc417ef0a0c5 100644
--- a/include/net/netfilter/nf_tables.h
+++ b/include/net/netfilter/nf_tables.h
@@ -1124,7 +1124,6 @@ struct nft_flowtable {
 	u32				genmask:2,
 					use:30;
 	u64				handle;
-	char				*dev_name[NFT_FLOWTABLE_DEVICE_MAX];
 	/* runtime data below here */
 	struct nf_hook_ops		*ops ____cacheline_aligned;
 	struct nf_flowtable		data;
diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index d41fa2c82f14..54a4f75ff9da 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -5324,8 +5324,6 @@ static int nf_tables_flowtable_parse_hook(const struct nft_ctx *ctx,
 		flowtable->ops[i].priv		= &flowtable->data;
 		flowtable->ops[i].hook		= flowtable->data.type->hook;
 		flowtable->ops[i].dev		= dev_array[i];
-		flowtable->dev_name[i]		= kstrdup(dev_array[i]->name,
-							  GFP_KERNEL);
 	}
 
 	return err;
@@ -5483,10 +5481,8 @@ static int nf_tables_newflowtable(struct net *net, struct sock *nlsk,
 err6:
 	i = flowtable->ops_len;
 err5:
-	for (k = i - 1; k >= 0; k--) {
-		kfree(flowtable->dev_name[k]);
+	for (k = i - 1; k >= 0; k--)
 		nf_unregister_net_hook(net, &flowtable->ops[k]);
-	}
 
 	kfree(flowtable->ops);
 err4:
@@ -5585,9 +5581,10 @@ static int nf_tables_fill_flowtable_info(struct sk_buff *skb, struct net *net,
 		goto nla_put_failure;
 
 	for (i = 0; i < flowtable->ops_len; i++) {
-		if (flowtable->dev_name[i][0] &&
-		    nla_put_string(skb, NFTA_DEVICE_NAME,
-				   flowtable->dev_name[i]))
+		const struct net_device *dev = READ_ONCE(flowtable->ops[i].dev);
+
+		if (dev &&
+		    nla_put_string(skb, NFTA_DEVICE_NAME, dev->name))
 			goto nla_put_failure;
 	}
 	nla_nest_end(skb, nest_devs);
@@ -5829,7 +5826,6 @@ static void nft_flowtable_event(unsigned long event, struct net_device *dev,
 			continue;
 
 		nf_unregister_net_hook(dev_net(dev), &flowtable->ops[i]);
-		flowtable->dev_name[i][0] = '\0';
 		flowtable->ops[i].dev = NULL;
 		break;
 	}
-- 
2.11.0

^ permalink raw reply related

* [PATCH 8/9] netfilter: conntrack: dccp: treat SYNC/SYNCACK as invalid if no prior state
From: Pablo Neira Ayuso @ 2018-07-24 16:31 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev
In-Reply-To: <20180724163133.14586-1-pablo@netfilter.org>

From: Florian Westphal <fw@strlen.de>

When first DCCP packet is SYNC or SYNCACK, we insert a new conntrack
that has an un-initialized timeout value, i.e. such entry could be
reaped at any time.

Mark them as INVALID and only ignore SYNC/SYNCACK when connection had
an old state.

Reported-by: syzbot+6f18401420df260e37ed@syzkaller.appspotmail.com
Signed-off-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/netfilter/nf_conntrack_proto_dccp.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/net/netfilter/nf_conntrack_proto_dccp.c b/net/netfilter/nf_conntrack_proto_dccp.c
index abe647d5b8c6..9ce6336d1e55 100644
--- a/net/netfilter/nf_conntrack_proto_dccp.c
+++ b/net/netfilter/nf_conntrack_proto_dccp.c
@@ -243,14 +243,14 @@ dccp_state_table[CT_DCCP_ROLE_MAX + 1][DCCP_PKT_SYNCACK + 1][CT_DCCP_MAX + 1] =
 		 * We currently ignore Sync packets
 		 *
 		 *	sNO, sRQ, sRS, sPO, sOP, sCR, sCG, sTW */
-			sIG, sIG, sIG, sIG, sIG, sIG, sIG, sIG,
+			sIV, sIG, sIG, sIG, sIG, sIG, sIG, sIG,
 		},
 		[DCCP_PKT_SYNCACK] = {
 		/*
 		 * We currently ignore SyncAck packets
 		 *
 		 *	sNO, sRQ, sRS, sPO, sOP, sCR, sCG, sTW */
-			sIG, sIG, sIG, sIG, sIG, sIG, sIG, sIG,
+			sIV, sIG, sIG, sIG, sIG, sIG, sIG, sIG,
 		},
 	},
 	[CT_DCCP_ROLE_SERVER] = {
@@ -371,14 +371,14 @@ dccp_state_table[CT_DCCP_ROLE_MAX + 1][DCCP_PKT_SYNCACK + 1][CT_DCCP_MAX + 1] =
 		 * We currently ignore Sync packets
 		 *
 		 *	sNO, sRQ, sRS, sPO, sOP, sCR, sCG, sTW */
-			sIG, sIG, sIG, sIG, sIG, sIG, sIG, sIG,
+			sIV, sIG, sIG, sIG, sIG, sIG, sIG, sIG,
 		},
 		[DCCP_PKT_SYNCACK] = {
 		/*
 		 * We currently ignore SyncAck packets
 		 *
 		 *	sNO, sRQ, sRS, sPO, sOP, sCR, sCG, sTW */
-			sIG, sIG, sIG, sIG, sIG, sIG, sIG, sIG,
+			sIV, sIG, sIG, sIG, sIG, sIG, sIG, sIG,
 		},
 	},
 };
-- 
2.11.0

^ permalink raw reply related

* [PATCH 7/9] netfilter: nf_tables: don't allow to rename to already-pending name
From: Pablo Neira Ayuso @ 2018-07-24 16:31 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev
In-Reply-To: <20180724163133.14586-1-pablo@netfilter.org>

From: Florian Westphal <fw@strlen.de>

Its possible to rename two chains to the same name in one
transaction:

nft add chain t c1
nft add chain t c2
nft 'rename chain t c1 c3;rename chain t c2 c3'

This creates two chains named 'c3'.

Appears to be harmless, both chains can still be deleted both
by name or handle, but, nevertheless, its a bug.

Walk transaction log and also compare vs. the pending renames.

Both chains can still be deleted, but nevertheless it is a bug as
we don't allow to create chains with identical names, so we should
prevent this from happening-by-rename too.

Signed-off-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/netfilter/nf_tables_api.c | 42 +++++++++++++++++++++++++++++-------------
 1 file changed, 29 insertions(+), 13 deletions(-)

diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index 91230d713190..d7b9748e338e 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -1598,7 +1598,6 @@ static int nf_tables_updchain(struct nft_ctx *ctx, u8 genmask, u8 policy,
 	struct nft_base_chain *basechain;
 	struct nft_stats *stats = NULL;
 	struct nft_chain_hook hook;
-	const struct nlattr *name;
 	struct nf_hook_ops *ops;
 	struct nft_trans *trans;
 	int err;
@@ -1646,12 +1645,11 @@ static int nf_tables_updchain(struct nft_ctx *ctx, u8 genmask, u8 policy,
 			return PTR_ERR(stats);
 	}
 
+	err = -ENOMEM;
 	trans = nft_trans_alloc(ctx, NFT_MSG_NEWCHAIN,
 				sizeof(struct nft_trans_chain));
-	if (trans == NULL) {
-		free_percpu(stats);
-		return -ENOMEM;
-	}
+	if (trans == NULL)
+		goto err;
 
 	nft_trans_chain_stats(trans) = stats;
 	nft_trans_chain_update(trans) = true;
@@ -1661,19 +1659,37 @@ static int nf_tables_updchain(struct nft_ctx *ctx, u8 genmask, u8 policy,
 	else
 		nft_trans_chain_policy(trans) = -1;
 
-	name = nla[NFTA_CHAIN_NAME];
-	if (nla[NFTA_CHAIN_HANDLE] && name) {
-		nft_trans_chain_name(trans) =
-			nla_strdup(name, GFP_KERNEL);
-		if (!nft_trans_chain_name(trans)) {
-			kfree(trans);
-			free_percpu(stats);
-			return -ENOMEM;
+	if (nla[NFTA_CHAIN_HANDLE] &&
+	    nla[NFTA_CHAIN_NAME]) {
+		struct nft_trans *tmp;
+		char *name;
+
+		err = -ENOMEM;
+		name = nla_strdup(nla[NFTA_CHAIN_NAME], GFP_KERNEL);
+		if (!name)
+			goto err;
+
+		err = -EEXIST;
+		list_for_each_entry(tmp, &ctx->net->nft.commit_list, list) {
+			if (tmp->msg_type == NFT_MSG_NEWCHAIN &&
+			    tmp->ctx.table == table &&
+			    nft_trans_chain_update(tmp) &&
+			    nft_trans_chain_name(tmp) &&
+			    strcmp(name, nft_trans_chain_name(tmp)) == 0) {
+				kfree(name);
+				goto err;
+			}
 		}
+
+		nft_trans_chain_name(trans) = name;
 	}
 	list_add_tail(&trans->list, &ctx->net->nft.commit_list);
 
 	return 0;
+err:
+	free_percpu(stats);
+	kfree(trans);
+	return err;
 }
 
 static int nf_tables_newchain(struct net *net, struct sock *nlsk,
-- 
2.11.0

^ permalink raw reply related

* [PATCH 6/9] netfilter: nf_tables: fix memory leaks on chain rename
From: Pablo Neira Ayuso @ 2018-07-24 16:31 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev
In-Reply-To: <20180724163133.14586-1-pablo@netfilter.org>

From: Florian Westphal <fw@strlen.de>

The new name is stored in the transaction metadata, on commit,
the pointers to the old and new names are swapped.

Therefore in abort and commit case we have to free the
pointer in the chain_trans container.

In commit case, the pointer can be used by another cpu that
is currently dumping the renamed chain, thus kfree needs to
happen after waiting for rcu readers to complete.

Fixes: b7263e071a ("netfilter: nf_tables: Allow chain name of up to 255 chars")
Signed-off-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/netfilter/nf_tables_api.c | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index 200da08524ae..91230d713190 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -6087,6 +6087,9 @@ static void nft_commit_release(struct nft_trans *trans)
 	case NFT_MSG_DELTABLE:
 		nf_tables_table_destroy(&trans->ctx);
 		break;
+	case NFT_MSG_NEWCHAIN:
+		kfree(nft_trans_chain_name(trans));
+		break;
 	case NFT_MSG_DELCHAIN:
 		nf_tables_chain_destroy(&trans->ctx);
 		break;
@@ -6316,13 +6319,15 @@ static int nf_tables_commit(struct net *net, struct sk_buff *skb)
 			nf_tables_table_notify(&trans->ctx, NFT_MSG_DELTABLE);
 			break;
 		case NFT_MSG_NEWCHAIN:
-			if (nft_trans_chain_update(trans))
+			if (nft_trans_chain_update(trans)) {
 				nft_chain_commit_update(trans);
-			else
+				nf_tables_chain_notify(&trans->ctx, NFT_MSG_NEWCHAIN);
+				/* trans destroyed after rcu grace period */
+			} else {
 				nft_clear(net, trans->ctx.chain);
-
-			nf_tables_chain_notify(&trans->ctx, NFT_MSG_NEWCHAIN);
-			nft_trans_destroy(trans);
+				nf_tables_chain_notify(&trans->ctx, NFT_MSG_NEWCHAIN);
+				nft_trans_destroy(trans);
+			}
 			break;
 		case NFT_MSG_DELCHAIN:
 			nft_chain_del(trans->ctx.chain);
@@ -6472,7 +6477,7 @@ static int __nf_tables_abort(struct net *net)
 		case NFT_MSG_NEWCHAIN:
 			if (nft_trans_chain_update(trans)) {
 				free_percpu(nft_trans_chain_stats(trans));
-
+				kfree(nft_trans_chain_name(trans));
 				nft_trans_destroy(trans);
 			} else {
 				trans->ctx.table->use--;
-- 
2.11.0

^ permalink raw reply related

* [PATCH 5/9] netfilter: nf_tables: free flow table struct too
From: Pablo Neira Ayuso @ 2018-07-24 16:31 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev
In-Reply-To: <20180724163133.14586-1-pablo@netfilter.org>

From: Florian Westphal <fw@strlen.de>

Fixes: 3b49e2e94e6ebb ("netfilter: nf_tables: add flow table netlink frontend")
Signed-off-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/netfilter/nf_tables_api.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index 54a4f75ff9da..200da08524ae 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -5784,6 +5784,7 @@ static void nf_tables_flowtable_destroy(struct nft_flowtable *flowtable)
 	kfree(flowtable->name);
 	flowtable->data.type->free(&flowtable->data);
 	module_put(flowtable->data.type->owner);
+	kfree(flowtable);
 }
 
 static int nf_tables_fill_gen_info(struct sk_buff *skb, struct net *net,
-- 
2.11.0

^ permalink raw reply related

* [PATCH 3/9] netfilter: nft_set_rbtree: fix panic when destroying set by GC
From: Pablo Neira Ayuso @ 2018-07-24 16:31 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev
In-Reply-To: <20180724163133.14586-1-pablo@netfilter.org>

From: Taehee Yoo <ap420073@gmail.com>

This patch fixes below.
1. check null pointer of rb_next.
 rb_next can return null. so null check routine should be added.
2. add rcu_barrier in destroy routine.
 GC uses call_rcu to remove elements. but all elements should be
 removed before destroying set and chains. so that rcu_barrier is added.

test script:
   %cat test.nft
   table inet aa {
	   map map1 {
		   type ipv4_addr : verdict; flags interval, timeout;
		   elements = {
			   0-1 : jump a0,
			   3-4 : jump a0,
			   6-7 : jump a0,
			   9-10 : jump a0,
			   12-13 : jump a0,
			   15-16 : jump a0,
			   18-19 : jump a0,
			   21-22 : jump a0,
			   24-25 : jump a0,
			   27-28 : jump a0,
		   }
		   timeout 1s;
	   }
	   chain a0 {
	   }
   }
   flush ruleset
   table inet aa {
	   map map1 {
		   type ipv4_addr : verdict; flags interval, timeout;
		   elements = {
			   0-1 : jump a0,
			   3-4 : jump a0,
			   6-7 : jump a0,
			   9-10 : jump a0,
			   12-13 : jump a0,
			   15-16 : jump a0,
			   18-19 : jump a0,
			   21-22 : jump a0,
			   24-25 : jump a0,
			   27-28 : jump a0,
		   }
		   timeout 1s;
	   }
	   chain a0 {
	   }
   }
   flush ruleset

splat looks like:
[ 2402.419838] kasan: GPF could be caused by NULL-ptr deref or user memory access
[ 2402.428433] general protection fault: 0000 [#1] SMP DEBUG_PAGEALLOC KASAN PTI
[ 2402.429343] CPU: 1 PID: 1350 Comm: kworker/1:1 Not tainted 4.18.0-rc2+ #1
[ 2402.429343] Hardware name: To be filled by O.E.M. To be filled by O.E.M./Aptio CRB, BIOS 5.6.5 03/23/2017
[ 2402.429343] Workqueue: events_power_efficient nft_rbtree_gc [nft_set_rbtree]
[ 2402.429343] RIP: 0010:rb_next+0x1e/0x130
[ 2402.429343] Code: e9 de f2 ff ff 0f 1f 80 00 00 00 00 41 55 48 89 fa 41 54 55 53 48 c1 ea 03 48 b8 00 00 00 0
[ 2402.429343] RSP: 0018:ffff880105f77678 EFLAGS: 00010296
[ 2402.429343] RAX: dffffc0000000000 RBX: ffff8801143e3428 RCX: 1ffff1002287c69c
[ 2402.429343] RDX: 0000000000000000 RSI: 0000000000000004 RDI: 0000000000000000
[ 2402.429343] RBP: 0000000000000000 R08: ffffed0016aabc24 R09: ffffed0016aabc24
[ 2402.429343] R10: 0000000000000001 R11: ffffed0016aabc23 R12: 0000000000000000
[ 2402.429343] R13: ffff8800b6933388 R14: dffffc0000000000 R15: ffff8801143e3440
[ 2402.534486] kasan: CONFIG_KASAN_INLINE enabled
[ 2402.534212] FS:  0000000000000000(0000) GS:ffff88011b600000(0000) knlGS:0000000000000000
[ 2402.534212] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 2402.534212] CR2: 0000000000863008 CR3: 00000000a3c16000 CR4: 00000000001006e0
[ 2402.534212] Call Trace:
[ 2402.534212]  nft_rbtree_gc+0x2b5/0x5f0 [nft_set_rbtree]
[ 2402.534212]  process_one_work+0xc1b/0x1ee0
[ 2402.540329] kasan: GPF could be caused by NULL-ptr deref or user memory access
[ 2402.534212]  ? _raw_spin_unlock_irq+0x29/0x40
[ 2402.534212]  ? pwq_dec_nr_in_flight+0x3e0/0x3e0
[ 2402.534212]  ? set_load_weight+0x270/0x270
[ 2402.534212]  ? __schedule+0x6ea/0x1fb0
[ 2402.534212]  ? __sched_text_start+0x8/0x8
[ 2402.534212]  ? save_trace+0x320/0x320
[ 2402.534212]  ? sched_clock_local+0xe2/0x150
[ 2402.534212]  ? find_held_lock+0x39/0x1c0
[ 2402.534212]  ? worker_thread+0x35f/0x1150
[ 2402.534212]  ? lock_contended+0xe90/0xe90
[ 2402.534212]  ? __lock_acquire+0x4520/0x4520
[ 2402.534212]  ? do_raw_spin_unlock+0xb1/0x350
[ 2402.534212]  ? do_raw_spin_trylock+0x111/0x1b0
[ 2402.534212]  ? do_raw_spin_lock+0x1f0/0x1f0
[ 2402.534212]  worker_thread+0x169/0x1150

Fixes: 8d8540c4f5e0("netfilter: nft_set_rbtree: add timeout support")
Signed-off-by: Taehee Yoo <ap420073@gmail.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/netfilter/nft_set_rbtree.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/net/netfilter/nft_set_rbtree.c b/net/netfilter/nft_set_rbtree.c
index 1f8f257cb518..9873d734b494 100644
--- a/net/netfilter/nft_set_rbtree.c
+++ b/net/netfilter/nft_set_rbtree.c
@@ -381,7 +381,7 @@ static void nft_rbtree_gc(struct work_struct *work)
 
 		gcb = nft_set_gc_batch_check(set, gcb, GFP_ATOMIC);
 		if (!gcb)
-			goto out;
+			break;
 
 		atomic_dec(&set->nelems);
 		nft_set_gc_batch_add(gcb, rbe);
@@ -390,10 +390,12 @@ static void nft_rbtree_gc(struct work_struct *work)
 			rbe = rb_entry(prev, struct nft_rbtree_elem, node);
 			atomic_dec(&set->nelems);
 			nft_set_gc_batch_add(gcb, rbe);
+			prev = NULL;
 		}
 		node = rb_next(node);
+		if (!node)
+			break;
 	}
-out:
 	if (gcb) {
 		for (i = 0; i < gcb->head.cnt; i++) {
 			rbe = gcb->elems[i];
@@ -440,6 +442,7 @@ static void nft_rbtree_destroy(const struct nft_set *set)
 	struct rb_node *node;
 
 	cancel_delayed_work_sync(&priv->gc_work);
+	rcu_barrier();
 	while ((node = priv->root.rb_node) != NULL) {
 		rb_erase(node, &priv->root);
 		rbe = rb_entry(node, struct nft_rbtree_elem, node);
-- 
2.11.0

^ permalink raw reply related

* [PATCH 1/9] netfilter: nf_tables: fix jumpstack depth validation
From: Pablo Neira Ayuso @ 2018-07-24 16:31 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev
In-Reply-To: <20180724163133.14586-1-pablo@netfilter.org>

From: Taehee Yoo <ap420073@gmail.com>

The level of struct nft_ctx is updated by nf_tables_check_loops().  That
is used to validate jumpstack depth. But jumpstack validation routine
doesn't update and validate recursively.  So, in some cases, chain depth
can be bigger than the NFT_JUMP_STACK_SIZE.

After this patch, The jumpstack validation routine is located in the
nft_chain_validate(). When new rules or new set elements are added, the
nft_table_validate() is called by the nf_tables_newrule and the
nf_tables_newsetelem. The nft_table_validate() calls the
nft_chain_validate() that visit all their children chains recursively.
So it can update depth of chain certainly.

Reproducer:
   %cat ./test.sh
   #!/bin/bash
   nft add table ip filter
   nft add chain ip filter input { type filter hook input priority 0\; }
   for ((i=0;i<20;i++)); do
	nft add chain ip filter a$i
   done

   nft add rule ip filter input jump a1

   for ((i=0;i<10;i++)); do
	nft add rule ip filter a$i jump a$((i+1))
   done

   for ((i=11;i<19;i++)); do
	nft add rule ip filter a$i jump a$((i+1))
   done

   nft add rule ip filter a10 jump a11

Result:
[  253.931782] WARNING: CPU: 1 PID: 0 at net/netfilter/nf_tables_core.c:186 nft_do_chain+0xacc/0xdf0 [nf_tables]
[  253.931915] Modules linked in: nf_tables nfnetlink ip_tables x_tables
[  253.932153] CPU: 1 PID: 0 Comm: swapper/1 Not tainted 4.18.0-rc3+ #48
[  253.932153] RIP: 0010:nft_do_chain+0xacc/0xdf0 [nf_tables]
[  253.932153] Code: 83 f8 fb 0f 84 c7 00 00 00 e9 d0 00 00 00 83 f8 fd 74 0e 83 f8 ff 0f 84 b4 00 00 00 e9 bd 00 00 00 83 bd 64 fd ff ff 0f 76 09 <0f> 0b 31 c0 e9 bc 02 00 00 44 8b ad 64 fd
[  253.933807] RSP: 0018:ffff88011b807570 EFLAGS: 00010212
[  253.933807] RAX: 00000000fffffffd RBX: ffff88011b807660 RCX: 0000000000000000
[  253.933807] RDX: 0000000000000010 RSI: ffff880112b39d78 RDI: ffff88011b807670
[  253.933807] RBP: ffff88011b807850 R08: ffffed0023700ece R09: ffffed0023700ecd
[  253.933807] R10: ffff88011b80766f R11: ffffed0023700ece R12: ffff88011b807898
[  253.933807] R13: ffff880112b39d80 R14: ffff880112b39d60 R15: dffffc0000000000
[  253.933807] FS:  0000000000000000(0000) GS:ffff88011b800000(0000) knlGS:0000000000000000
[  253.933807] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[  253.933807] CR2: 00000000014f1008 CR3: 000000006b216000 CR4: 00000000001006e0
[  253.933807] Call Trace:
[  253.933807]  <IRQ>
[  253.933807]  ? sched_clock_cpu+0x132/0x170
[  253.933807]  ? __nft_trace_packet+0x180/0x180 [nf_tables]
[  253.933807]  ? sched_clock_cpu+0x132/0x170
[  253.933807]  ? debug_show_all_locks+0x290/0x290
[  253.933807]  ? __lock_acquire+0x4835/0x4af0
[  253.933807]  ? inet_ehash_locks_alloc+0x1a0/0x1a0
[  253.933807]  ? unwind_next_frame+0x159e/0x1840
[  253.933807]  ? __read_once_size_nocheck.constprop.4+0x5/0x10
[  253.933807]  ? nft_do_chain_ipv4+0x197/0x1e0 [nf_tables]
[  253.933807]  ? nft_do_chain+0x5/0xdf0 [nf_tables]
[  253.933807]  nft_do_chain_ipv4+0x197/0x1e0 [nf_tables]
[  253.933807]  ? nft_do_chain_arp+0xb0/0xb0 [nf_tables]
[  253.933807]  ? __lock_is_held+0x9d/0x130
[  253.933807]  nf_hook_slow+0xc4/0x150
[  253.933807]  ip_local_deliver+0x28b/0x380
[  253.933807]  ? ip_call_ra_chain+0x3e0/0x3e0
[  253.933807]  ? ip_rcv_finish+0x1610/0x1610
[  253.933807]  ip_rcv+0xbcc/0xcc0
[  253.933807]  ? debug_show_all_locks+0x290/0x290
[  253.933807]  ? ip_local_deliver+0x380/0x380
[  253.933807]  ? __lock_is_held+0x9d/0x130
[  253.933807]  ? ip_local_deliver+0x380/0x380
[  253.933807]  __netif_receive_skb_core+0x1c9c/0x2240

Signed-off-by: Taehee Yoo <ap420073@gmail.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 include/net/netfilter/nf_tables.h |  4 ++--
 net/netfilter/nf_tables_api.c     | 11 ++++-------
 net/netfilter/nft_immediate.c     |  3 +++
 net/netfilter/nft_lookup.c        | 13 +++++++++++--
 4 files changed, 20 insertions(+), 11 deletions(-)

diff --git a/include/net/netfilter/nf_tables.h b/include/net/netfilter/nf_tables.h
index 08c005ce56e9..4e82a4c49912 100644
--- a/include/net/netfilter/nf_tables.h
+++ b/include/net/netfilter/nf_tables.h
@@ -150,6 +150,7 @@ static inline void nft_data_debug(const struct nft_data *data)
  *	@portid: netlink portID of the original message
  *	@seq: netlink sequence number
  *	@family: protocol family
+ *	@level: depth of the chains
  *	@report: notify via unicast netlink message
  */
 struct nft_ctx {
@@ -160,6 +161,7 @@ struct nft_ctx {
 	u32				portid;
 	u32				seq;
 	u8				family;
+	u8				level;
 	bool				report;
 };
 
@@ -865,7 +867,6 @@ enum nft_chain_flags {
  *	@table: table that this chain belongs to
  *	@handle: chain handle
  *	@use: number of jump references to this chain
- *	@level: length of longest path to this chain
  *	@flags: bitmask of enum nft_chain_flags
  *	@name: name of the chain
  */
@@ -878,7 +879,6 @@ struct nft_chain {
 	struct nft_table		*table;
 	u64				handle;
 	u32				use;
-	u16				level;
 	u8				flags:6,
 					genmask:2;
 	char				*name;
diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index 896d4a36081d..d41fa2c82f14 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -75,6 +75,7 @@ static void nft_ctx_init(struct nft_ctx *ctx,
 {
 	ctx->net	= net;
 	ctx->family	= family;
+	ctx->level	= 0;
 	ctx->table	= table;
 	ctx->chain	= chain;
 	ctx->nla   	= nla;
@@ -2384,6 +2385,9 @@ int nft_chain_validate(const struct nft_ctx *ctx, const struct nft_chain *chain)
 	struct nft_rule *rule;
 	int err;
 
+	if (ctx->level == NFT_JUMP_STACK_SIZE)
+		return -EMLINK;
+
 	list_for_each_entry(rule, &chain->rules, list) {
 		if (!nft_is_active_next(ctx->net, rule))
 			continue;
@@ -6837,13 +6841,6 @@ int nft_validate_register_store(const struct nft_ctx *ctx,
 			err = nf_tables_check_loops(ctx, data->verdict.chain);
 			if (err < 0)
 				return err;
-
-			if (ctx->chain->level + 1 >
-			    data->verdict.chain->level) {
-				if (ctx->chain->level + 1 == NFT_JUMP_STACK_SIZE)
-					return -EMLINK;
-				data->verdict.chain->level = ctx->chain->level + 1;
-			}
 		}
 
 		return 0;
diff --git a/net/netfilter/nft_immediate.c b/net/netfilter/nft_immediate.c
index 15adf8ca82c3..0777a93211e2 100644
--- a/net/netfilter/nft_immediate.c
+++ b/net/netfilter/nft_immediate.c
@@ -98,6 +98,7 @@ static int nft_immediate_validate(const struct nft_ctx *ctx,
 				  const struct nft_data **d)
 {
 	const struct nft_immediate_expr *priv = nft_expr_priv(expr);
+	struct nft_ctx *pctx = (struct nft_ctx *)ctx;
 	const struct nft_data *data;
 	int err;
 
@@ -109,9 +110,11 @@ static int nft_immediate_validate(const struct nft_ctx *ctx,
 	switch (data->verdict.code) {
 	case NFT_JUMP:
 	case NFT_GOTO:
+		pctx->level++;
 		err = nft_chain_validate(ctx, data->verdict.chain);
 		if (err < 0)
 			return err;
+		pctx->level--;
 		break;
 	default:
 		break;
diff --git a/net/netfilter/nft_lookup.c b/net/netfilter/nft_lookup.c
index 42e6fadf1417..c2a1d84cdfc4 100644
--- a/net/netfilter/nft_lookup.c
+++ b/net/netfilter/nft_lookup.c
@@ -155,7 +155,9 @@ static int nft_lookup_validate_setelem(const struct nft_ctx *ctx,
 				       struct nft_set_elem *elem)
 {
 	const struct nft_set_ext *ext = nft_set_elem_ext(set, elem->priv);
+	struct nft_ctx *pctx = (struct nft_ctx *)ctx;
 	const struct nft_data *data;
+	int err;
 
 	if (nft_set_ext_exists(ext, NFT_SET_EXT_FLAGS) &&
 	    *nft_set_ext_flags(ext) & NFT_SET_ELEM_INTERVAL_END)
@@ -165,10 +167,17 @@ static int nft_lookup_validate_setelem(const struct nft_ctx *ctx,
 	switch (data->verdict.code) {
 	case NFT_JUMP:
 	case NFT_GOTO:
-		return nft_chain_validate(ctx, data->verdict.chain);
+		pctx->level++;
+		err = nft_chain_validate(ctx, data->verdict.chain);
+		if (err < 0)
+			return err;
+		pctx->level--;
+		break;
 	default:
-		return 0;
+		break;
 	}
+
+	return 0;
 }
 
 static int nft_lookup_validate(const struct nft_ctx *ctx,
-- 
2.11.0

^ permalink raw reply related

* [PATCH 2/9] netfilter: nft_set_hash: add rcu_barrier() in the nft_rhash_destroy()
From: Pablo Neira Ayuso @ 2018-07-24 16:31 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev
In-Reply-To: <20180724163133.14586-1-pablo@netfilter.org>

From: Taehee Yoo <ap420073@gmail.com>

GC of set uses call_rcu() to destroy elements.
So that elements would be destroyed after destroying sets and chains.
But, elements should be destroyed before destroying sets and chains.
In order to wait calling call_rcu(), a rcu_barrier() is added.

In order to test correctly, below patch should be applied.
https://patchwork.ozlabs.org/patch/940883/

test scripts:
   %cat test.nft
   table ip aa {
	   map map1 {
		   type ipv4_addr : verdict; flags timeout;
		   elements = {
			   0 : jump a0,
			   1 : jump a0,
			   2 : jump a0,
			   3 : jump a0,
			   4 : jump a0,
			   5 : jump a0,
			   6 : jump a0,
			   7 : jump a0,
			   8 : jump a0,
			   9 : jump a0,
		   }
		   timeout 1s;
	   }
	   chain a0 {
	   }
   }
   flush ruleset

   [ ... ]

   table ip aa {
	   map map1 {
		   type ipv4_addr : verdict; flags timeout;
		   elements = {
			   0 : jump a0,
			   1 : jump a0,
			   2 : jump a0,
			   3 : jump a0,
			   4 : jump a0,
			   5 : jump a0,
			   6 : jump a0,
			   7 : jump a0,
			   8 : jump a0,
			   9 : jump a0,
		   }
		   timeout 1s;
	   }
	   chain a0 {
	   }
   }
   flush ruleset

Splat looks like:
[  200.795603] kernel BUG at net/netfilter/nf_tables_api.c:1363!
[  200.806944] invalid opcode: 0000 [#1] SMP DEBUG_PAGEALLOC KASAN PTI
[  200.812253] CPU: 1 PID: 1582 Comm: nft Not tainted 4.17.0+ #24
[  200.820297] Hardware name: To be filled by O.E.M. To be filled by O.E.M./Aptio CRB, BIOS 5.6.5 07/08/2015
[  200.830309] RIP: 0010:nf_tables_chain_destroy.isra.34+0x62/0x240 [nf_tables]
[  200.838317] Code: 43 50 85 c0 74 26 48 8b 45 00 48 8b 4d 08 ba 54 05 00 00 48 c7 c6 60 6d 29 c0 48 c7 c7 c0 65 29 c0
4c 8b 40 08 e8 58 e5 fd f8 <0f> 0b 48 89 da 48 b8 00 00 00 00 00 fc ff
[  200.860366] RSP: 0000:ffff880118dbf4d0 EFLAGS: 00010282
[  200.866354] RAX: 0000000000000061 RBX: ffff88010cdeaf08 RCX: 0000000000000000
[  200.874355] RDX: 0000000000000061 RSI: 0000000000000008 RDI: ffffed00231b7e90
[  200.882361] RBP: ffff880118dbf4e8 R08: ffffed002373bcfb R09: ffffed002373bcfa
[  200.890354] R10: 0000000000000000 R11: ffffed002373bcfb R12: dead000000000200
[  200.898356] R13: dead000000000100 R14: ffffffffbb62af38 R15: dffffc0000000000
[  200.906354] FS:  00007fefc31fd700(0000) GS:ffff88011b800000(0000) knlGS:0000000000000000
[  200.915533] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[  200.922355] CR2: 0000557f1c8e9128 CR3: 0000000106880000 CR4: 00000000001006e0
[  200.930353] Call Trace:
[  200.932351]  ? nf_tables_commit+0x26f6/0x2c60 [nf_tables]
[  200.939525]  ? nf_tables_setelem_notify.constprop.49+0x1a0/0x1a0 [nf_tables]
[  200.947525]  ? nf_tables_delchain+0x6e0/0x6e0 [nf_tables]
[  200.952383]  ? nft_add_set_elem+0x1700/0x1700 [nf_tables]
[  200.959532]  ? nla_parse+0xab/0x230
[  200.963529]  ? nfnetlink_rcv_batch+0xd06/0x10d0 [nfnetlink]
[  200.968384]  ? nfnetlink_net_init+0x130/0x130 [nfnetlink]
[  200.975525]  ? debug_show_all_locks+0x290/0x290
[  200.980363]  ? debug_show_all_locks+0x290/0x290
[  200.986356]  ? sched_clock_cpu+0x132/0x170
[  200.990352]  ? find_held_lock+0x39/0x1b0
[  200.994355]  ? sched_clock_local+0x10d/0x130
[  200.999531]  ? memset+0x1f/0x40

Fixes: 9d0982927e79 ("netfilter: nft_hash: add support for timeouts")
Signed-off-by: Taehee Yoo <ap420073@gmail.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/netfilter/nft_set_hash.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/netfilter/nft_set_hash.c b/net/netfilter/nft_set_hash.c
index 72ef35b51cac..90c3e7e6cacb 100644
--- a/net/netfilter/nft_set_hash.c
+++ b/net/netfilter/nft_set_hash.c
@@ -387,6 +387,7 @@ static void nft_rhash_destroy(const struct nft_set *set)
 	struct nft_rhash *priv = nft_set_priv(set);
 
 	cancel_delayed_work_sync(&priv->gc_work);
+	rcu_barrier();
 	rhashtable_free_and_destroy(&priv->ht, nft_rhash_elem_destroy,
 				    (void *)set);
 }
-- 
2.11.0

^ permalink raw reply related

* [PATCH 0/9] Netfilter fixes for net
From: Pablo Neira Ayuso @ 2018-07-24 16:31 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev

Hi David,

The following patchset contains Netfilter fixes for net:

1) Make sure we don't go over the maximum jump stack boundary,
   from Taehee Yoo.

2) Missing rcu_barrier() in hash and rbtree sets, also from Taehee.

3) Missing check to nul-node in rbtree timeout routine, from Taehee.

4) Use dev->name from flowtable to fix a memleak, from Florian.

5) Oneliner to free flowtable object on removal, from Florian.

6) Memleak in chain rename transaction, again from Florian.

7) Don't allow two chains to use the same name in the same
   transaction, from Florian.

8) handle DCCP SYNC/SYNCACK as invalid, this triggers an
   uninitialized timer in conntrack reported by syzbot, from Florian.

9) Fix leak in case netlink_dump_start() fails, from Florian.

You can pull these changes from:

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

Thanks.

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

The following changes since commit 1992d99882afda6dc17f9d49c06150856a91282f:

  net/smc: take sock lock in smc_ioctl() (2018-07-16 14:45:13 -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 90fd131afc565159c9e0ea742f082b337e10f8c6:

  netfilter: nf_tables: move dumper state allocation into ->start (2018-07-24 00:36:33 +0200)

----------------------------------------------------------------
Florian Westphal (6):
      netfilter: nf_tables: use dev->name directly
      netfilter: nf_tables: free flow table struct too
      netfilter: nf_tables: fix memory leaks on chain rename
      netfilter: nf_tables: don't allow to rename to already-pending name
      netfilter: conntrack: dccp: treat SYNC/SYNCACK as invalid if no prior state
      netfilter: nf_tables: move dumper state allocation into ->start

Taehee Yoo (3):
      netfilter: nf_tables: fix jumpstack depth validation
      netfilter: nft_set_hash: add rcu_barrier() in the nft_rhash_destroy()
      netfilter: nft_set_rbtree: fix panic when destroying set by GC

 include/net/netfilter/nf_tables.h       |   5 +-
 net/netfilter/nf_conntrack_proto_dccp.c |   8 +-
 net/netfilter/nf_tables_api.c           | 304 +++++++++++++++++---------------
 net/netfilter/nft_immediate.c           |   3 +
 net/netfilter/nft_lookup.c              |  13 +-
 net/netfilter/nft_set_hash.c            |   1 +
 net/netfilter/nft_set_rbtree.c          |   7 +-
 7 files changed, 191 insertions(+), 150 deletions(-)

^ permalink raw reply

* your photos
From: Roland @ 2018-07-24 10:34 UTC (permalink / raw)
  To: netdev

I would like to speak with the person that managing photos for your
company?

We provide image editing like – photos cutting out and retouching.

Enhancing your images is just a part of what we can do for your business.
Whether you’re an ecommerce
store or portrait photographer, real estate professional, or an e-Retailer,
we are your personal team
of photo editors that integrate seamlessly with your business.

Our mainly services are:

 . Cut out, masking, clipping path, deep etching, transparent background
 . Colour correction, black and white, light and shadows etc.
 . Dust cleaning, spot cleaning
 . Beauty retouching, skin retouching, face retouching, body retouching
 . Fashion/Beauty Image Retouching
 . Product image Retouching
 . Real estate image Retouching
 . Wedding & Event Album Design.
 . Restoration and repair old images
 . Vector Conversion
 . Portrait image Retouching

We can provide you editing test on your photos.
Please reply if you are interested.

Thanks,
Roland

^ permalink raw reply

* Re: [PATCH v6 18/18] crypto: Remove AHASH_REQUEST_ON_STACK
From: Joe Perches @ 2018-07-24 17:31 UTC (permalink / raw)
  To: Kees Cook, Herbert Xu
  Cc: Arnd Bergmann, Eric Biggers, Gustavo A. R. Silva, Alasdair Kergon,
	Rabin Vincent, Tim Chen, Rafael J. Wysocki, Pavel Machek,
	Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86,
	Philipp Reisner, Lars Ellenberg, Jens Axboe, Giovanni Cabiddu,
	Mike Snitzer, Paul Mackerras, Greg Kroah-Hartman, David Howells,
	Johannes Berg <joh
In-Reply-To: <20180724164936.37477-19-keescook@chromium.org>

On Tue, 2018-07-24 at 09:49 -0700, Kees Cook wrote:
> All users of AHASH_REQUEST_ON_STACK have been removed from the kernel, so
> drop it entirely so no VLAs get reintroduced by future users.

checkpatch has a test for that.
It could now be removed as well.
---
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 34e4683de7a3..a3517334d661 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -796,7 +796,7 @@ our $declaration_macros = qr{(?x:
 	(?:$Storage\s+)?(?:[A-Z_][A-Z0-9]*_){0,2}(?:DEFINE|DECLARE)(?:_[A-Z0-9]+){1,6}\s*\(|
 	(?:$Storage\s+)?[HLP]?LIST_HEAD\s*\(|
 	(?:$Storage\s+)?${Type}\s+uninitialized_var\s*\(|
-	(?:SKCIPHER_REQUEST|SHASH_DESC|AHASH_REQUEST)_ON_STACK\s*\(
+	(?:SKCIPHER_REQUEST|SHASH_DESC)_ON_STACK\s*\(
 )};

^ permalink raw reply related

* Re: [**EXTERNAL**] Re: VRF with enslaved L3 enabled bridge
From: D'Souza, Nelson @ 2018-07-24 16:08 UTC (permalink / raw)
  To: David Ahern, netdev@vger.kernel.org
In-Reply-To: <0A43C74F-A402-47A3-98A0-2700F767294F@ciena.com>

It's strange that enslaving eth1 -> br0 -> test-vrf does not work, but enslaving eth1->test-vrf works fine.

Nelson

On 7/24/18, 8:58 AM, "D'Souza, Nelson" <ndsouza@ciena.com> wrote:

    Thank you David, really appreciate the help. Most likely something specific to my environment.
    
    ip vrf id, does not report anything on my system. Here's the result after running the command.
    
    # ip vrf id
    #
    
    I'll follow up with a VM.
    
    Nelson
    
    On 7/24/18, 5:55 AM, "David Ahern" <dsa@cumulusnetworks.com> wrote:
    
        On 7/23/18 7:43 PM, D'Souza, Nelson wrote:
        > I copy and pasted the configs onto my device, but pings on test-vrf do not work in my setup. 
        > I'm essentially seeing the same issue as I reported before.
        > 
        > In this case, pings sent out on test-vrf (host ns) are received and replied to by the loopback interface (foo ns). Although the replies are seen at the test-vrf level, they are not locally delivered to the ping application.
        > 
        
        I just built v4.14.52 kernel and ran those commands - worked fine. It is
        something specific to your environment. Is your shell tied to a VRF --
        (ip vrf id)?
        
        After that, I suggest you create a VM running a newer distribution of
        your choice (Ubuntu 17.10 or newer, debian stretch with 4.14 kernel, or
        Fedora 26 or newer) and run the commands there.
        
    
    


^ permalink raw reply

* Re: [PATCH RFC/RFT net-next 00/17] net: Convert neighbor tables to per-namespace
From: David Miller @ 2018-07-24 17:14 UTC (permalink / raw)
  To: dsahern
  Cc: xiyou.wangcong, netdev, nikita.leshchenko, roopa, stephen, idosch,
	jiri, saeedm, alex.aring, linux-wpan, netfilter-devel,
	linux-kernel
In-Reply-To: <5021d874-8e99-6eba-f24b-4257c62d4457@gmail.com>

From: David Ahern <dsahern@gmail.com>
Date: Tue, 24 Jul 2018 09:14:01 -0600

> I get the impression there is no longer a strong resistance against
> moving the tables to per namespace, but deciding what is the right
> approach to handle backwards compatibility. Correct? Changing the
> accounting is inevitably going to be noticeable to some use case(s), but
> with sysctl settings it is a simple runtime update once the user knows
> to make the change.
> 
> neighbor entries round up to 512 byte allocations, so with the current
> gc_thresh defaults (128/512/1024) 512k can be consumed. Using those
> limits per namespace seems high which is why I suggested a per-namespace
> default of (16/32/64) which amounts to 32k per namespace limit by
> default. Open to other suggestions as well.

No objection from me about going to per-ns neigh tables.

About the defaults, I wonder if we can scale them to the amount of
memory given to the ns or something like that?  I bet this will better
match the intended use of the ns.

^ permalink raw reply

* Re: [PATCH v4] selftests: add headers_install to lib.mk
From: Shuah Khan @ 2018-07-24 17:10 UTC (permalink / raw)
  To: Anders Roxell, Masahiro Yamada, Michal Marek, Bamvor Zhang, brgl,
	Paolo Bonzini, Andrew Morton, Mike Rapoport, aarcange
  Cc: linux-kbuild, Linux Kernel Mailing List,
	open list:KERNEL SELFTEST FRAMEWORK, Networking, Shuah Khan
In-Reply-To: <CADYN=9J=hSFb2wCZ1+dXL=023p9y05MEGUPvH48WfPX7XDZvwQ@mail.gmail.com>

On 07/23/2018 02:49 PM, Anders Roxell wrote:
> On Thu, 7 Jun 2018 at 13:09, Anders Roxell <anders.roxell@linaro.org> wrote:
>>
>> If the kernel headers aren't installed we can't build all the tests.
>> Add a new make target rule 'khdr' in the file lib.mk to generate the
>> kernel headers and that gets include for every test-dir Makefile that
>> includes lib.mk If the testdir in turn have its own sub-dirs the
>> top_srcdir needs to be set to the linux-rootdir to be able to generate
>> the kernel headers.
>>
>> Signed-off-by: Anders Roxell <anders.roxell@linaro.org>
>> Reviewed-by: Fathi Boudra <fathi.boudra@linaro.org>
>> ---
>>  Makefile                                           | 14 +-------------
>>  scripts/subarch.include                            | 13 +++++++++++++
>>  tools/testing/selftests/android/Makefile           |  2 +-
>>  tools/testing/selftests/android/ion/Makefile       |  2 ++
>>  tools/testing/selftests/futex/functional/Makefile  |  1 +
>>  tools/testing/selftests/gpio/Makefile              |  7 ++-----
>>  tools/testing/selftests/kvm/Makefile               |  7 ++-----
>>  tools/testing/selftests/lib.mk                     | 12 ++++++++++++
>>  tools/testing/selftests/net/Makefile               |  1 +
>>  .../selftests/networking/timestamping/Makefile     |  1 +
>>  tools/testing/selftests/vm/Makefile                |  4 ----
>>  11 files changed, 36 insertions(+), 28 deletions(-)
>>  create mode 100644 scripts/subarch.include
>>
>> diff --git a/Makefile b/Makefile
>> index 6b9aea95ae3a..8050072300fa 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -286,19 +286,7 @@ KERNELRELEASE = $(shell cat include/config/kernel.release 2> /dev/null)
>>  KERNELVERSION = $(VERSION)$(if $(PATCHLEVEL),.$(PATCHLEVEL)$(if $(SUBLEVEL),.$(SUBLEVEL)))$(EXTRAVERSION)
>>  export VERSION PATCHLEVEL SUBLEVEL KERNELRELEASE KERNELVERSION
>>
>> -# SUBARCH tells the usermode build what the underlying arch is.  That is set
>> -# first, and if a usermode build is happening, the "ARCH=um" on the command
>> -# line overrides the setting of ARCH below.  If a native build is happening,
>> -# then ARCH is assigned, getting whatever value it gets normally, and
>> -# SUBARCH is subsequently ignored.
>> -
>> -SUBARCH := $(shell uname -m | sed -e s/i.86/x86/ -e s/x86_64/x86/ \
>> -                                 -e s/sun4u/sparc64/ \
>> -                                 -e s/arm.*/arm/ -e s/sa110/arm/ \
>> -                                 -e s/s390x/s390/ -e s/parisc64/parisc/ \
>> -                                 -e s/ppc.*/powerpc/ -e s/mips.*/mips/ \
>> -                                 -e s/sh[234].*/sh/ -e s/aarch64.*/arm64/ \
>> -                                 -e s/riscv.*/riscv/)
>> +include scripts/subarch.include

What is the reason for this SUBARCH block move to to scripts/subarch.include?
Is this necessary for adding headers install dependency to lib.mk?


>>
>>  # Cross compiling and selecting different set of gcc/bin-utils
>>  # ---------------------------------------------------------------------------
>> diff --git a/scripts/subarch.include b/scripts/subarch.include
>> new file mode 100644
>> index 000000000000..650682821126
>> --- /dev/null
>> +++ b/scripts/subarch.include
>> @@ -0,0 +1,13 @@
>> +# SUBARCH tells the usermode build what the underlying arch is.  That is set
>> +# first, and if a usermode build is happening, the "ARCH=um" on the command
>> +# line overrides the setting of ARCH below.  If a native build is happening,
>> +# then ARCH is assigned, getting whatever value it gets normally, and
>> +# SUBARCH is subsequently ignored.
>> +
>> +SUBARCH := $(shell uname -m | sed -e s/i.86/x86/ -e s/x86_64/x86/ \
>> +                                 -e s/sun4u/sparc64/ \
>> +                                 -e s/arm.*/arm/ -e s/sa110/arm/ \
>> +                                 -e s/s390x/s390/ -e s/parisc64/parisc/ \
>> +                                 -e s/ppc.*/powerpc/ -e s/mips.*/mips/ \
>> +                                 -e s/sh[234].*/sh/ -e s/aarch64.*/arm64/ \
>> +                                 -e s/riscv.*/riscv/)

thanks,
-- Shuah

^ permalink raw reply

* Re: [PATCH net-next] bnxt_en: Fix logic of forward the VF MAC address to PF in bnxt_vf_validate_set_mac
From: Vasundhara Volam @ 2018-07-24 16:01 UTC (permalink / raw)
  To: Michael Chan; +Cc: YueHaibing, David Miller, open list, Netdev
In-Reply-To: <CACKFLini64_5N7aVtPAVH3CBYCSsdHAJv35jimR18PJXeV6j5g@mail.gmail.com>

On Tue, Jul 24, 2018 at 1:01 PM, Michael Chan <michael.chan@broadcom.com> wrote:
>
> On Mon, Jul 23, 2018 at 10:24 PM, YueHaibing <yuehaibing@huawei.com> wrote:
> > Based on the comments,req->l2addr must match the VF MAC address
> > if firmware spec >= 1.2.2, mac_ok can be true.
> >
> > Signed-off-by: YueHaibing <yuehaibing@huawei.com>
> > ---
> >  drivers/net/ethernet/broadcom/bnxt/bnxt_sriov.c | 7 ++-----
> >  1 file changed, 2 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_sriov.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_sriov.c
> > index a649108..7925964 100644
> > --- a/drivers/net/ethernet/broadcom/bnxt/bnxt_sriov.c
> > +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_sriov.c
> > @@ -954,12 +954,9 @@ static int bnxt_vf_validate_set_mac(struct bnxt *bp, struct bnxt_vf_info *vf)
> >                 if (ether_addr_equal((const u8 *)req->l2_addr, vf->mac_addr))
> >                         mac_ok = true;
> >         } else if (is_valid_ether_addr(vf->vf_mac_addr)) {
> > -               if (ether_addr_equal((const u8 *)req->l2_addr, vf->vf_mac_addr))
> > +               if (ether_addr_equal((const u8 *)req->l2_addr, vf->vf_mac_addr) &&
> > +                   bp->hwrm_spec_code >= 0x10202)
> >                         mac_ok = true;
>
> I'm not sure if this is correct.  If firmware spec < 0x10202, the VF
> MAC address is not forwarded to the PF and so it doesn't have to match
> and mac_ok should still be true.  I think we are missing that
> condition with this patch.
>
> I need to let my colleague Vasundhara comment on this.  She is more
> familiar with this logic.
Yes Michael, you are right. Also, the plain else condition is to cover
a special case to allow VF to modify
it's own MAC when PF has not assigned a valid MAC address and HWRM
spec code > 0x10202.
>
> > -       } else if (bp->hwrm_spec_code < 0x10202) {
> > -               mac_ok = true;
> > -       } else {
> > -               mac_ok = true;
> >         }
> >         if (mac_ok)
> >                 return bnxt_hwrm_exec_fwd_resp(bp, vf, msg_size);
> > --
> > 2.7.0
> >
> >

^ permalink raw reply

* Re: [**EXTERNAL**] Re: VRF with enslaved L3 enabled bridge
From: D'Souza, Nelson @ 2018-07-24 15:58 UTC (permalink / raw)
  To: David Ahern, netdev@vger.kernel.org
In-Reply-To: <75195753-3180-aada-b0a5-98b1a0dd2f49@cumulusnetworks.com>

Thank you David, really appreciate the help. Most likely something specific to my environment.

ip vrf id, does not report anything on my system. Here's the result after running the command.

# ip vrf id
#

I'll follow up with a VM.

Nelson

On 7/24/18, 5:55 AM, "David Ahern" <dsa@cumulusnetworks.com> wrote:

    On 7/23/18 7:43 PM, D'Souza, Nelson wrote:
    > I copy and pasted the configs onto my device, but pings on test-vrf do not work in my setup. 
    > I'm essentially seeing the same issue as I reported before.
    > 
    > In this case, pings sent out on test-vrf (host ns) are received and replied to by the loopback interface (foo ns). Although the replies are seen at the test-vrf level, they are not locally delivered to the ping application.
    > 
    
    I just built v4.14.52 kernel and ran those commands - worked fine. It is
    something specific to your environment. Is your shell tied to a VRF --
    (ip vrf id)?
    
    After that, I suggest you create a VM running a newer distribution of
    your choice (Ubuntu 17.10 or newer, debian stretch with 4.14 kernel, or
    Fedora 26 or newer) and run the commands there.
    


^ permalink raw reply

* Re: [PATCH v3 bpf 3/3] bpf: Introduce BPF_ANNOTATE_KV_PAIR
From: Yonghong Song @ 2018-07-24 15:54 UTC (permalink / raw)
  To: Martin KaFai Lau, netdev; +Cc: Alexei Starovoitov, Daniel Borkmann, kernel-team
In-Reply-To: <20180724154022.3863907-4-kafai@fb.com>



On 7/24/18 8:40 AM, Martin KaFai Lau wrote:
> This patch introduces BPF_ANNOTATE_KV_PAIR to signal the
> bpf loader about the btf key_type and value_type of a bpf map.
> Please refer to the changes in test_btf_haskv.c for its usage.
> Both iproute2 and libbpf loader will then have the same
> convention to find out the map's btf_key_type_id and
> btf_value_type_id from a map's name.
> 
> Fixes: 8a138aed4a80 ("bpf: btf: Add BTF support to libbpf")
> Suggested-by: Daniel Borkmann <daniel@iogearbox.net>
> Signed-off-by: Martin KaFai Lau <kafai@fb.com>

Acked-by: Yonghong Song <yhs@fb.com>

^ permalink raw reply

* Re: [PATCH v3 bpf 2/3] bpf: Replace [u]int32_t and [u]int64_t in libbpf
From: Yonghong Song @ 2018-07-24 15:52 UTC (permalink / raw)
  To: Martin KaFai Lau, netdev; +Cc: Alexei Starovoitov, Daniel Borkmann, kernel-team
In-Reply-To: <20180724154022.3863907-3-kafai@fb.com>



On 7/24/18 8:40 AM, Martin KaFai Lau wrote:
> This patch replaces [u]int32_t and [u]int64_t usage with
> __[su]32 and __[su]64.  The same change goes for [u]int16_t
> and [u]int8_t.
> 
> Fixes: 8a138aed4a80 ("bpf: btf: Add BTF support to libbpf")
> Signed-off-by: Martin KaFai Lau <kafai@fb.com>

Acked-by: Yonghong Song <yhs@fb.com>

^ permalink raw reply

* Re: [PATCH v6 17/18] crypto: ccm: Remove VLA usage
From: Ard Biesheuvel @ 2018-07-24 16:57 UTC (permalink / raw)
  To: Kees Cook
  Cc: Herbert Xu, Arnd Bergmann, Eric Biggers, Gustavo A. R. Silva,
	Alasdair Kergon, Rabin Vincent, Tim Chen, Rafael J. Wysocki,
	Pavel Machek, Thomas Gleixner, Ingo Molnar, H. Peter Anvin,
	the arch/x86 maintainers, Philipp Reisner, Lars Ellenberg,
	Jens Axboe, Giovanni Cabiddu, Mike Snitzer, Paul Mackerras,
	Greg Kroah-Hartman <gregk
In-Reply-To: <20180724164936.37477-18-keescook@chromium.org>

On 24 July 2018 at 18:49, Kees Cook <keescook@chromium.org> wrote:
> From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>
> In the quest to remove all stack VLA usage from the kernel[1], this
> drops AHASH_REQUEST_ON_STACK by preallocated the ahash request area
> with the skcipher area (which are not used at the same time).
>

-EGRAMMAR

> [1] https://lkml.kernel.org/r/CA+55aFzCG-zNmZwX4A2FQpadafLfEzK6CC=qPXydAacU1RqZWA@mail.gmail.com
>
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Signed-off-by: Kees Cook <keescook@chromium.org>
> ---
>  crypto/ccm.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/crypto/ccm.c b/crypto/ccm.c
> index 0a083342ec8c..b242fd0d3262 100644
> --- a/crypto/ccm.c
> +++ b/crypto/ccm.c
> @@ -50,7 +50,10 @@ struct crypto_ccm_req_priv_ctx {
>         u32 flags;
>         struct scatterlist src[3];
>         struct scatterlist dst[3];
> -       struct skcipher_request skreq;
> +       union {
> +               struct ahash_request ahreq;
> +               struct skcipher_request skreq;
> +       };
>  };
>
>  struct cbcmac_tfm_ctx {
> @@ -181,7 +184,7 @@ static int crypto_ccm_auth(struct aead_request *req, struct scatterlist *plain,
>         struct crypto_ccm_req_priv_ctx *pctx = crypto_ccm_reqctx(req);
>         struct crypto_aead *aead = crypto_aead_reqtfm(req);
>         struct crypto_ccm_ctx *ctx = crypto_aead_ctx(aead);
> -       AHASH_REQUEST_ON_STACK(ahreq, ctx->mac);
> +       struct ahash_request *ahreq = &pctx->ahreq;
>         unsigned int assoclen = req->assoclen;
>         struct scatterlist sg[3];
>         u8 *odata = pctx->odata;
> @@ -427,7 +430,7 @@ static int crypto_ccm_init_tfm(struct crypto_aead *tfm)
>         crypto_aead_set_reqsize(
>                 tfm,
>                 align + sizeof(struct crypto_ccm_req_priv_ctx) +
> -               crypto_skcipher_reqsize(ctr));
> +               max(crypto_ahash_reqsize(mac), crypto_skcipher_reqsize(ctr)));
>
>         return 0;
>
> --
> 2.17.1
>

^ permalink raw reply

* selftests/bpf test_sockmap failure
From: Yonghong Song @ 2018-07-24 15:45 UTC (permalink / raw)
  To: John Fastabend, netdev; +Cc: Yonghong Song, Martin Lau

In one of our production machines, tools/testing/selftests/bpf
test_sockmap failed randomly like below:

...
[TEST 78]: (512, 1, 1, sendmsg, pass,apply 1,): rx thread exited with 
err 1. FAILED
...

...
[TEST 80]: (2, 1024, 256, sendmsg, pass,apply 1,): rx thread exited with 
err 1. FAILED
...

...
[TEST 83]: (100, 1, 5, sendpage, pass,apply 1,): rx thread exited with 
err 1. FAILED
...

...
[TEST 79]: (512, 1, 1, sendpage, pass,apply 1,): rx thread exited with 
err 1. FAILED
...

The command line is just `test_sockmap`. The machine has 80 cpus, 256G 
memory. The kernel is based on 4.16 but backported with latest bpf-next 
bpf changes.

The failed test number (78, 79, 80, or 83) is random. But they all share
similar characteristics:
    . the option rate is greater than one, i.e., more than one
      sendmsg/sendpage in the sender forked process.
    . The txmsg_apply is not 0

I debugged a little bit. It happens in msg_loop() function below
"unexpected timeout" path.

...
                         slct = select(max_fd + 1, &w, NULL, NULL, 
&timeout);
                         if (slct == -1) {
                                 perror("select()");
                                 clock_gettime(CLOCK_MONOTONIC, &s->end);
                                 goto out_errno;
                         } else if (!slct) {
                                 if (opt->verbose)
                                         fprintf(stderr, "unexpected 
timeout\n");
                                 errno = -EIO;
                                 clock_gettime(CLOCK_MONOTONIC, &s->end);
                                 goto out_errno;
                         }
...

It appears that when the error happens, the receive process does not 
receive all bytes sent from the send process and eventually times out.

Has anybody seen this issue as well?
John, any comments on this failure?

Thanks,

Yonghong

^ permalink raw reply

* Re: [PATCH] net: sort Kconfig menu items alphabetically
From: Jon Mason @ 2018-07-24 15:43 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: David S. Miller, Florian Fainelli, Rasesh Mody, Sudarsana Kalluru,
	Dept-GELinuxNICDev, netdev, linux-kernel
In-Reply-To: <153237967407.172440.6754268418710104431.stgit@bhelgaas-glaptop.roam.corp.google.com>

On Mon, Jul 23, 2018 at 04:01:14PM -0500, Bjorn Helgaas wrote:
> From: Bjorn Helgaas <bhelgaas@google.com>
> 
> 6c541b4595a2 ("net: ethernet: Sort Kconfig sourcing alphabetically")
> sorted Kconfig sourcing based on directory names, but in a couple cases,
> the menu item text is quite different from the directory name and is not
> sorted correctly:
> 
>   drivers/net/ethernet/neterion/Kconfig    => "Exar devices"

Stupid question, does it make more sense to clarify the names in the
Kconfig to more naturally follow what people are expecting?  In the
case of Neterion, it was standalone for several years, then aquired by
Exar and shutdown in 11 months without ever making any Exar branded
adatpers.  Users, if they still exist, would probably think of it as
Neterion and not Exar (as there have been no follow-on adapters and I
don't believe any plans to ever do so).

So, this might be better as a rename of the Kconfig to something like
"Neterion (Exar) devices".  Of course, there is a fair amount of Exar
names in the Kconfig.  If you agree, I can do a clean-up of this to
match the expectations.

Thoughts?

Thanks,
Jon

>   drivers/net/ethernet/brocade/Kconfig     => "QLogic BR-series devices"
> 
> Move these entries so the Kconfig menu items are sorted.
> 
> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
> ---
>  drivers/net/ethernet/Kconfig |    4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/Kconfig b/drivers/net/ethernet/Kconfig
> index af766fd61151..b21febba458e 100644
> --- a/drivers/net/ethernet/Kconfig
> +++ b/drivers/net/ethernet/Kconfig
> @@ -34,7 +34,6 @@ source "drivers/net/ethernet/arc/Kconfig"
>  source "drivers/net/ethernet/atheros/Kconfig"
>  source "drivers/net/ethernet/aurora/Kconfig"
>  source "drivers/net/ethernet/broadcom/Kconfig"
> -source "drivers/net/ethernet/brocade/Kconfig"
>  source "drivers/net/ethernet/cadence/Kconfig"
>  source "drivers/net/ethernet/calxeda/Kconfig"
>  source "drivers/net/ethernet/cavium/Kconfig"
> @@ -71,6 +70,7 @@ config DNET
>  source "drivers/net/ethernet/dec/Kconfig"
>  source "drivers/net/ethernet/dlink/Kconfig"
>  source "drivers/net/ethernet/emulex/Kconfig"
> +source "drivers/net/ethernet/neterion/Kconfig"
>  source "drivers/net/ethernet/ezchip/Kconfig"
>  source "drivers/net/ethernet/faraday/Kconfig"
>  source "drivers/net/ethernet/freescale/Kconfig"
> @@ -81,7 +81,6 @@ source "drivers/net/ethernet/huawei/Kconfig"
>  source "drivers/net/ethernet/i825xx/Kconfig"
>  source "drivers/net/ethernet/ibm/Kconfig"
>  source "drivers/net/ethernet/intel/Kconfig"
> -source "drivers/net/ethernet/neterion/Kconfig"
>  source "drivers/net/ethernet/xscale/Kconfig"
>  
>  config JME
> @@ -160,6 +159,7 @@ config ETHOC
>  source "drivers/net/ethernet/packetengines/Kconfig"
>  source "drivers/net/ethernet/pasemi/Kconfig"
>  source "drivers/net/ethernet/qlogic/Kconfig"
> +source "drivers/net/ethernet/brocade/Kconfig"
>  source "drivers/net/ethernet/qualcomm/Kconfig"
>  source "drivers/net/ethernet/rdc/Kconfig"
>  source "drivers/net/ethernet/realtek/Kconfig"
> 

^ permalink raw reply

* Re: [PATCH net-next v3 0/8] A fix and a few improvements on mvneta
From: David Miller @ 2018-07-24 16:50 UTC (permalink / raw)
  To: gregory.clement
  Cc: linux-kernel, netdev, thomas.petazzoni, linux-arm-kernel, jason,
	andrew, sebastian.hesselbarth, yelena, nadavh, mw, dima,
	antoine.tenart, miquel.raynal, maxime.chevallier
In-Reply-To: <87h8ko7ozf.fsf@bootlin.com>

From: Gregory CLEMENT <gregory.clement@bootlin.com>
Date: Tue, 24 Jul 2018 15:16:52 +0200

> I saw in patchwork, the status was "Accepted" (for example the 1st patch
> [1]). However the series is not in your git branches and usually you
> also answered to the series to inform that you have applied it. So I
> would like to know what is the status of the series.

Sorry about that, I'll take a look and try to figure out what happened
and fix it up.

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox