From: Zihan Xi 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: stable@vger.kernel.org Reported-by: Vega Assisted-by: Codex:gpt-5.4 Signed-off-by: Zihan Xi Signed-off-by: Ren Wei --- 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