netfilter: Can't fail and free after table replacement

commit c58dd2dd443c26d856a168db108a0cd11c285bf3 upstream.

All xtables variants suffer from the defect that the copy_to_user()
to copy the counters to user memory may fail after the table has
already been exchanged and thus exposed. Return an error at this
point will result in freeing the already exposed table. Any
subsequent packet processing will result in a kernel panic.

We can't copy the counters before exposing the new tables as we
want provide the counter state after the old table has been
unhooked. Therefore convert this into a silent error.

Cc: Florian Westphal <fw@strlen.de>
Signed-off-by: Thomas Graf <tgraf@suug.ch>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

diff --git a/net/bridge/netfilter/ebtables.c b/net/bridge/netfilter/ebtables.c
index 3d110c4fc7870dda1d6efcb00f9169f819dc6526..6651a7797d46aca846d833086990c953bfdbfa63 100644 (file)
--- a/net/bridge/netfilter/ebtables.c
+++ b/net/bridge/netfilter/ebtables.c
@@ -1044,10 +1044,9 @@ static int do_replace_finish(struct net *net, struct ebt_replace *repl,
        if (repl->num_counters &&
           copy_to_user(repl->counters, counterstmp,
           repl->num_counters * sizeof(struct ebt_counter))) {
-               ret = -EFAULT;
+               /* Silent error, can't fail, new table is already in place */
+               net_warn_ratelimited("ebtables: counters copy to user failed while replacing table\n");
        }
-       else
-               ret = 0;
 
        /* decrease module count and free resources */
        EBT_ENTRY_ITERATE(table->entries, table->entries_size,

diff --git a/net/ipv4/netfilter/arp_tables.c b/net/ipv4/netfilter/arp_tables.c
index 85a4f21aac1ad117f740d40da7123a663fdfefa8..c8abe31961eda38c087a0057c53d678998ecc09a 100644 (file)
--- a/net/ipv4/netfilter/arp_tables.c
+++ b/net/ipv4/netfilter/arp_tables.c
@@ -1039,8 +1039,10 @@ static int __do_replace(struct net *net, const char *name,
 
        xt_free_table_info(oldinfo);
        if (copy_to_user(counters_ptr, counters,
-                        sizeof(struct xt_counters) * num_counters) != 0)
-               ret = -EFAULT;
+                        sizeof(struct xt_counters) * num_counters) != 0) {
+               /* Silent error, can't fail, new table is already in place */
+               net_warn_ratelimited("arptables: counters copy to user failed while replacing table\n");
+       }
        vfree(counters);
        xt_table_unlock(t);
        return ret;

diff --git a/net/ipv4/netfilter/ip_tables.c b/net/ipv4/netfilter/ip_tables.c
index d23118d95ff9291401396953d094391db0e2e44e..651c10774d588c2484d4e140f79fb30d29d7b4df 100644 (file)
--- a/net/ipv4/netfilter/ip_tables.c
+++ b/net/ipv4/netfilter/ip_tables.c
@@ -1226,8 +1226,10 @@ __do_replace(struct net *net, const char *name, unsigned int valid_hooks,
 
        xt_free_table_info(oldinfo);
        if (copy_to_user(counters_ptr, counters,
-                        sizeof(struct xt_counters) * num_counters) != 0)
-               ret = -EFAULT;
+                        sizeof(struct xt_counters) * num_counters) != 0) {
+               /* Silent error, can't fail, new table is already in place */
+               net_warn_ratelimited("iptables: counters copy to user failed while replacing table\n");
+       }
        vfree(counters);
        xt_table_unlock(t);
        return ret;

diff --git a/net/ipv6/netfilter/ip6_tables.c b/net/ipv6/netfilter/ip6_tables.c
index 44400c216dc6361e4607fe9af3e2999639766a11..89a4e4ddd8bbeede5ee4d0a610652779e9684394 100644 (file)
--- a/net/ipv6/netfilter/ip6_tables.c
+++ b/net/ipv6/netfilter/ip6_tables.c
@@ -1236,8 +1236,10 @@ __do_replace(struct net *net, const char *name, unsigned int valid_hooks,
 
        xt_free_table_info(oldinfo);
        if (copy_to_user(counters_ptr, counters,
-                        sizeof(struct xt_counters) * num_counters) != 0)
-               ret = -EFAULT;
+                        sizeof(struct xt_counters) * num_counters) != 0) {
+               /* Silent error, can't fail, new table is already in place */
+               net_warn_ratelimited("ip6tables: counters copy to user failed while replacing table\n");
+       }
        vfree(counters);
        xt_table_unlock(t);
        return ret;