[PATCH nf 1/1] netfilter: ebtables: avoid unbounded counter allocations

From: Zihan Xi <[email protected]>

EBT_SO_SET_COUNTERS copies only the fixed ebt_replace header before
calling do_update_counters().  update_counters() checks the user supplied
length against the user supplied num_counters, but do_update_counters()
then allocates the temporary counter array before it looks up the table
and before it compares num_counters with the table's real entry count.

EBT_SO_SET_ENTRIES has the same issue when userspace asks for the old
counters back.  do_replace_finish() used repl->num_counters to allocate
the temporary counter array before it looked up the table and before it
validated repl->num_counters against t->private->nentries.

A caller with CAP_NET_ADMIN in its network namespace can therefore pass a
large num_counters value and force a multi-gigabyte vmalloc attempt even
when the named table does not exist or the counter count does not match
the table.  The request is rejected only after the allocation attempt.

Look up the table and validate num_counters against t->private->nentries
before allocating temporary counter arrays.  This keeps valid counter
updates and replacements unchanged while bogus table names and
counter-count mismatches are rejected before any user-sized counter
allocation is attempted.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: [email protected]
Reported-by: Vega <[email protected]>
Assisted-by: Codex:gpt-5.4
Signed-off-by: Zihan Xi <[email protected]>
Signed-off-by: Ren Wei <[email protected]>
---
 net/bridge/netfilter/ebtables.c | 34 ++++++++++++++++-----------------
 1 file changed, 17 insertions(+), 17 deletions(-)

diff --git a/net/bridge/netfilter/ebtables.c b/net/bridge/netfilter/ebtables.c
index 96c9a8f57c87..81496ebf92a9 100644
--- a/net/bridge/netfilter/ebtables.c
+++ b/net/bridge/netfilter/ebtables.c
@@ -1017,16 +1017,6 @@ static int do_replace_finish(struct net *net, struct ebt_replace *repl,
 	struct ebt_table_info *table;
 	struct ebt_table *t;
 
-	/* the user wants counters back
-	 * the check on the size is done later, when we have the lock
-	 */
-	if (repl->num_counters) {
-		counterstmp = vmalloc_array(repl->num_counters,
-					    sizeof(*counterstmp));
-		if (!counterstmp)
-			return -ENOMEM;
-	}
-
 	newinfo->chainstack = NULL;
 	ret = ebt_verify_pointers(repl, newinfo);
 	if (ret != 0)
@@ -1053,6 +1043,15 @@ static int do_replace_finish(struct net *net, struct ebt_replace *repl,
 		goto free_unlock;
 	}
 
+	if (repl->num_counters) {
+		counterstmp = vmalloc_array(repl->num_counters,
+					    sizeof(*counterstmp));
+		if (!counterstmp) {
+			ret = -ENOMEM;
+			goto free_unlock;
+		}
+	}
+
 	/* we have the mutex lock, so no danger in reading this pointer */
 	table = t->private;
 	/* make sure the table can only be rmmod'ed if it contains no rules */
@@ -1386,25 +1385,27 @@ static int do_update_counters(struct net *net, const char *name,
 			      unsigned int num_counters, unsigned int len)
 {
 	int i, ret;
-	struct ebt_counter *tmp;
+	struct ebt_counter *tmp = NULL;
 	struct ebt_table *t;
 
 	if (num_counters == 0)
 		return -EINVAL;
 
-	tmp = vmalloc_array(num_counters, sizeof(*tmp));
-	if (!tmp)
-		return -ENOMEM;
-
 	t = find_table_lock(net, name, &ret, &ebt_mutex);
 	if (!t)
-		goto free_tmp;
+		return ret;
 
 	if (num_counters != t->private->nentries) {
 		ret = -EINVAL;
 		goto unlock_mutex;
 	}
 
+	tmp = vmalloc_array(num_counters, sizeof(*tmp));
+	if (!tmp) {
+		ret = -ENOMEM;
+		goto unlock_mutex;
+	}
+
 	if (copy_from_user(tmp, counters,
 			   array_size(num_counters, sizeof(*counters)))) {
 		ret = -EFAULT;
@@ -1422,7 +1423,6 @@ static int do_update_counters(struct net *net, const char *name,
 	ret = 0;
 unlock_mutex:
 	mutex_unlock(&ebt_mutex);
-free_tmp:
 	vfree(tmp);
 	return ret;
 }
-- 
2.43.0