[NETFILTER]: Fix possible overflow in netfilters do_replace()

Linux Kernel Mailing List <[email protected]> Tue, 13 Jun 2006 19:59:27 GMT
Newsgroups gmane.linux.kernel.commits.2-4
Message-ID <[email protected]>
commit 2a5b7391b19f2831dc03bf443cdde48e8fa997e1
tree 289ae6877b4c0e4a77c74ec827cb8e9e4d7ac1aa
parent 13c55afa98dfba769941448e59269959737af057
author Kirill Korotaev <[email protected]> Mon, 29 May 2006 16:10:39 -0700
committer David S. Miller <[email protected]> Mon, 29 May 2006 16:10:39 -0700

[NETFILTER]: Fix possible overflow in netfilters do_replace()

netfilter's do_replace() can overflow on addition within SMP_ALIGN()
and/or on multiplication by NR_CPUS, resulting in a buffer overflow on
the copy_from_user().  In practice, the overflow on addition is
triggerable on all systems, whereas the multiplication one might require
much physical memory to be present due to the check above.  Either is
sufficient to overwrite arbitrary amounts of kernel memory.

I really hate adding the same check to all 4 versions of do_replace(),
but the code is duplicate...

Found by Solar Designer during security audit of OpenVZ.org

Signed-Off-By: Kirill Korotaev <[email protected]>
Signed-Off-By: Solar Designer <[email protected]>
Signed-off-by: Patrck McHardy <[email protected]>
Signed-off-by: David S. Miller <[email protected]>

 net/ipv4/netfilter/arp_tables.c |    7 +++++++
 net/ipv4/netfilter/ip_tables.c  |    7 +++++++
 net/ipv6/netfilter/ip6_tables.c |    7 +++++++
 3 files changed, 21 insertions(+)

diff --git a/net/ipv4/netfilter/arp_tables.c b/net/ipv4/netfilter/arp_tables.c
index bc2e3a5..d831080 100644
--- a/net/ipv4/netfilter/arp_tables.c
+++ b/net/ipv4/netfilter/arp_tables.c
@@ -871,6 +871,13 @@ static int do_replace(void *user, unsign
 	if (len != sizeof(tmp) + tmp.size)
 		return -ENOPROTOOPT;
 
+	/* overflow check */
+	if (tmp.size >= (INT_MAX - sizeof(struct arpt_table_info)) / NR_CPUS -
+			SMP_CACHE_BYTES)
+		return -ENOMEM;
+	if (tmp.num_counters >= INT_MAX / sizeof(struct arpt_counters))
+		return -ENOMEM;
+
 	/* Pedantry: prevent them from hitting BUG() in vmalloc.c --RR */
 	if ((SMP_ALIGN(tmp.size) >> PAGE_SHIFT) + 2 > num_physpages)
 		return -ENOMEM;
diff --git a/net/ipv4/netfilter/ip_tables.c b/net/ipv4/netfilter/ip_tables.c
index c945a9e..416687b 100644
--- a/net/ipv4/netfilter/ip_tables.c
+++ b/net/ipv4/netfilter/ip_tables.c
@@ -1066,6 +1066,13 @@ do_replace(void *user, unsigned int len)
 	if (len != sizeof(tmp) + tmp.size)
 		return -ENOPROTOOPT;
 
+	/* overflow check */
+	if (tmp.size >= (INT_MAX - sizeof(struct ipt_table_info)) / NR_CPUS -
+			SMP_CACHE_BYTES)
+		return -ENOMEM;
+	if (tmp.num_counters >= INT_MAX / sizeof(struct ipt_counters))
+		return -ENOMEM;
+
 	/* Pedantry: prevent them from hitting BUG() in vmalloc.c --RR */
 	if ((SMP_ALIGN(tmp.size) >> PAGE_SHIFT) + 2 > num_physpages)
 		return -ENOMEM;
diff --git a/net/ipv6/netfilter/ip6_tables.c b/net/ipv6/netfilter/ip6_tables.c
index 31ead31..02127ba 100644
--- a/net/ipv6/netfilter/ip6_tables.c
+++ b/net/ipv6/netfilter/ip6_tables.c
@@ -1151,6 +1151,13 @@ do_replace(void *user, unsigned int len)
 	if ((SMP_ALIGN(tmp.size) >> PAGE_SHIFT) + 2 > num_physpages)
 		return -ENOMEM;
 
+	/* overflow check */
+	if (tmp.size >= (INT_MAX - sizeof(struct ip6t_table_info)) / NR_CPUS -
+			SMP_CACHE_BYTES)
+		return -ENOMEM;
+	if (tmp.num_counters >= INT_MAX / sizeof(struct ip6t_counters))
+		return -ENOMEM;
+
 	newinfo = vmalloc(sizeof(struct ip6t_table_info)
 			  + SMP_ALIGN(tmp.size) * smp_num_cpus);
 	if (!newinfo)