[PATCH nf-next 0/3] netfilter: ebtables: shashiko nitpicks

[PATCH nf-next 0/3] netfilter: ebtables: shashiko nitpicks

[Florian Westphal]

Three academic bug fixes for ebtables, based on sashiko
drive-by reviews.

All of these "bugs" have existed forever, kets treat this
as cleanups.

Florian Westphal (3):
  netfilter: ebtables: zero chainstack array
  netfilter: ebtables: account compat ebt_table_info to kmemcg
  netfilter: ebtables: bound num_counters in do_update_counters()

 net/bridge/netfilter/ebtables.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

-- 
2.55.0

[PATCH nf-next 1/3] netfilter: ebtables: zero chainstack array

[Florian Westphal]

shashiko reports:
 looking at ebtables table
 translation, could a sparse cpu_possible_mask lead to an uninitialized pointer
 free?

 If cpu_possible_mask is sparse (for example, CPU 0 and CPU 2 are possible,
 but CPU 1 is not), the allocation loop skips CPU 1. If vmalloc_node() fails at
 CPU 2, the cleanup loop will blindly decrement and call vfree() on
 newinfo->chainstack[1].

Not a real-world bug, such allocation isn't expected to fail
in the first place.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Florian Westphal <fw@strlen.de>
---
 net/bridge/netfilter/ebtables.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/net/bridge/netfilter/ebtables.c b/net/bridge/netfilter/ebtables.c
index 042d31278713..66c407cffa16 100644
--- a/net/bridge/netfilter/ebtables.c
+++ b/net/bridge/netfilter/ebtables.c
@@ -923,8 +923,7 @@ static int translate_table(struct net *net, const char *name,
 		 * if an error occurs
 		 */
 		newinfo->chainstack =
-			vmalloc_array(nr_cpu_ids,
-				      sizeof(*(newinfo->chainstack)));
+			vcalloc(nr_cpu_ids, sizeof(*(newinfo->chainstack)));
 		if (!newinfo->chainstack)
 			return -ENOMEM;
 		for_each_possible_cpu(i) {
-- 
2.55.0

[PATCH nf-next 2/3] netfilter: ebtables: account compat ebt_table_info to kmemcg

[Florian Westphal]

sashiko says:
 does compat_do_replace() bypass memory cgroup accounting?

This code is on the chopping block, but lets fix this up
for -stable sake.

Fixes: e2c8d550a973 ("netfilter: ebtables: account ebt_table_info to kmemcg")
Signed-off-by: Florian Westphal <fw@strlen.de>
---
 net/bridge/netfilter/ebtables.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/bridge/netfilter/ebtables.c b/net/bridge/netfilter/ebtables.c
index 66c407cffa16..7238e48d51a1 100644
--- a/net/bridge/netfilter/ebtables.c
+++ b/net/bridge/netfilter/ebtables.c
@@ -2299,7 +2299,7 @@ static int compat_do_replace(struct net *net, sockptr_t arg, unsigned int len)
 	}
 
 	countersize = COUNTER_OFFSET(tmp.nentries) * nr_cpu_ids;
-	newinfo = vmalloc(sizeof(*newinfo) + countersize);
+	newinfo = __vmalloc(sizeof(*newinfo) + countersize, GFP_KERNEL_ACCOUNT);
 	if (!newinfo)
 		return -ENOMEM;
 
@@ -2308,7 +2308,7 @@ static int compat_do_replace(struct net *net, sockptr_t arg, unsigned int len)
 
 	memset(&state, 0, sizeof(state));
 
-	newinfo->entries = vmalloc(tmp.entries_size);
+	newinfo->entries = __vmalloc(tmp.entries_size, GFP_KERNEL_ACCOUNT);
 	if (!newinfo->entries) {
 		ret = -ENOMEM;
 		goto free_newinfo;
-- 
2.55.0

[PATCH nf-next 3/3] netfilter: ebtables: bound num_counters in do_update_counters()

[Florian Westphal]

sashiko says:
 it appears the exact same
 pattern remains unfixed in the sibling function do_update_counters().

Fixes: 43ae85af154b ("netfilter: ebtables: bound num_counters like nentries in do_replace()")
Signed-off-by: Florian Westphal <fw@strlen.de>
---
 net/bridge/netfilter/ebtables.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/net/bridge/netfilter/ebtables.c b/net/bridge/netfilter/ebtables.c
index 7238e48d51a1..80008664ebe5 100644
--- a/net/bridge/netfilter/ebtables.c
+++ b/net/bridge/netfilter/ebtables.c
@@ -1390,6 +1390,9 @@ static int do_update_counters(struct net *net, const char *name,
 	if (num_counters == 0)
 		return -EINVAL;
 
+	if (num_counters >= MAX_EBT_ENTRIES)
+		return -ENOMEM;
+
 	tmp = vmalloc_array(num_counters, sizeof(*tmp));
 	if (!tmp)
 		return -ENOMEM;
-- 
2.55.0

Re: [PATCH nf-next 0/3] netfilter: ebtables: shashiko nitpicks

[Florian Westphal]

Florian Westphal <fw@strlen.de> wrote:
> Three academic bug fixes for ebtables, based on sashiko
> drive-by reviews.
> 
> All of these "bugs" have existed forever, kets treat this
> as cleanups.
> 
> Florian Westphal (3):
>   netfilter: ebtables: zero chainstack array
>   netfilter: ebtables: account compat ebt_table_info to kmemcg
>   netfilter: ebtables: bound num_counters in do_update_counters()

Many more drive-bys here.  I don't think this is worth our time.

I propose to axe arptables CONFIG_COMPAT in nf-next
now.

Then, axe ebtables-compat next querter and finally rest of xt-32bit compat
after that.

All the xt COMPAT flavours are already restricted to the initial
namespace.