diff options
author | Dave Jones <davej@codemonkey.org.uk> | 2015-05-19 20:55:17 -0400 |
---|---|---|
committer | Pablo Neira Ayuso <pablo@netfilter.org> | 2015-05-20 07:46:49 -0400 |
commit | 1086bbe97a074844188c6c988fa0b1a98c3ccbb9 (patch) | |
tree | c4b90cdf9b27072391ec62aa06408e1a57baed1e | |
parent | 3bfe049807c240344b407e3cfb74544927359817 (diff) |
netfilter: ensure number of counters is >0 in do_replace()
After improving setsockopt() coverage in trinity, I started triggering
vmalloc failures pretty reliably from this code path:
warn_alloc_failed+0xe9/0x140
__vmalloc_node_range+0x1be/0x270
vzalloc+0x4b/0x50
__do_replace+0x52/0x260 [ip_tables]
do_ipt_set_ctl+0x15d/0x1d0 [ip_tables]
nf_setsockopt+0x65/0x90
ip_setsockopt+0x61/0xa0
raw_setsockopt+0x16/0x60
sock_common_setsockopt+0x14/0x20
SyS_setsockopt+0x71/0xd0
It turns out we don't validate that the num_counters field in the
struct we pass in from userspace is initialized.
The same problem also exists in ebtables, arptables, ipv6, and the
compat variants.
Signed-off-by: Dave Jones <davej@codemonkey.org.uk>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
-rw-r--r-- | net/bridge/netfilter/ebtables.c | 4 | ||||
-rw-r--r-- | net/ipv4/netfilter/arp_tables.c | 6 | ||||
-rw-r--r-- | net/ipv4/netfilter/ip_tables.c | 6 | ||||
-rw-r--r-- | net/ipv6/netfilter/ip6_tables.c | 6 |
4 files changed, 22 insertions, 0 deletions
diff --git a/net/bridge/netfilter/ebtables.c b/net/bridge/netfilter/ebtables.c index 91180a7fc943..24c7c96bf5f8 100644 --- a/net/bridge/netfilter/ebtables.c +++ b/net/bridge/netfilter/ebtables.c | |||
@@ -1117,6 +1117,8 @@ static int do_replace(struct net *net, const void __user *user, | |||
1117 | return -ENOMEM; | 1117 | return -ENOMEM; |
1118 | if (tmp.num_counters >= INT_MAX / sizeof(struct ebt_counter)) | 1118 | if (tmp.num_counters >= INT_MAX / sizeof(struct ebt_counter)) |
1119 | return -ENOMEM; | 1119 | return -ENOMEM; |
1120 | if (tmp.num_counters == 0) | ||
1121 | return -EINVAL; | ||
1120 | 1122 | ||
1121 | tmp.name[sizeof(tmp.name) - 1] = 0; | 1123 | tmp.name[sizeof(tmp.name) - 1] = 0; |
1122 | 1124 | ||
@@ -2159,6 +2161,8 @@ static int compat_copy_ebt_replace_from_user(struct ebt_replace *repl, | |||
2159 | return -ENOMEM; | 2161 | return -ENOMEM; |
2160 | if (tmp.num_counters >= INT_MAX / sizeof(struct ebt_counter)) | 2162 | if (tmp.num_counters >= INT_MAX / sizeof(struct ebt_counter)) |
2161 | return -ENOMEM; | 2163 | return -ENOMEM; |
2164 | if (tmp.num_counters == 0) | ||
2165 | return -EINVAL; | ||
2162 | 2166 | ||
2163 | memcpy(repl, &tmp, offsetof(struct ebt_replace, hook_entry)); | 2167 | memcpy(repl, &tmp, offsetof(struct ebt_replace, hook_entry)); |
2164 | 2168 | ||
diff --git a/net/ipv4/netfilter/arp_tables.c b/net/ipv4/netfilter/arp_tables.c index 13bfe84bf3ca..a61200754f4b 100644 --- a/net/ipv4/netfilter/arp_tables.c +++ b/net/ipv4/netfilter/arp_tables.c | |||
@@ -1075,6 +1075,9 @@ static int do_replace(struct net *net, const void __user *user, | |||
1075 | /* overflow check */ | 1075 | /* overflow check */ |
1076 | if (tmp.num_counters >= INT_MAX / sizeof(struct xt_counters)) | 1076 | if (tmp.num_counters >= INT_MAX / sizeof(struct xt_counters)) |
1077 | return -ENOMEM; | 1077 | return -ENOMEM; |
1078 | if (tmp.num_counters == 0) | ||
1079 | return -EINVAL; | ||
1080 | |||
1078 | tmp.name[sizeof(tmp.name)-1] = 0; | 1081 | tmp.name[sizeof(tmp.name)-1] = 0; |
1079 | 1082 | ||
1080 | newinfo = xt_alloc_table_info(tmp.size); | 1083 | newinfo = xt_alloc_table_info(tmp.size); |
@@ -1499,6 +1502,9 @@ static int compat_do_replace(struct net *net, void __user *user, | |||
1499 | return -ENOMEM; | 1502 | return -ENOMEM; |
1500 | if (tmp.num_counters >= INT_MAX / sizeof(struct xt_counters)) | 1503 | if (tmp.num_counters >= INT_MAX / sizeof(struct xt_counters)) |
1501 | return -ENOMEM; | 1504 | return -ENOMEM; |
1505 | if (tmp.num_counters == 0) | ||
1506 | return -EINVAL; | ||
1507 | |||
1502 | tmp.name[sizeof(tmp.name)-1] = 0; | 1508 | tmp.name[sizeof(tmp.name)-1] = 0; |
1503 | 1509 | ||
1504 | newinfo = xt_alloc_table_info(tmp.size); | 1510 | newinfo = xt_alloc_table_info(tmp.size); |
diff --git a/net/ipv4/netfilter/ip_tables.c b/net/ipv4/netfilter/ip_tables.c index c69db7fa25ee..2d0e265fef6e 100644 --- a/net/ipv4/netfilter/ip_tables.c +++ b/net/ipv4/netfilter/ip_tables.c | |||
@@ -1262,6 +1262,9 @@ do_replace(struct net *net, const void __user *user, unsigned int len) | |||
1262 | /* overflow check */ | 1262 | /* overflow check */ |
1263 | if (tmp.num_counters >= INT_MAX / sizeof(struct xt_counters)) | 1263 | if (tmp.num_counters >= INT_MAX / sizeof(struct xt_counters)) |
1264 | return -ENOMEM; | 1264 | return -ENOMEM; |
1265 | if (tmp.num_counters == 0) | ||
1266 | return -EINVAL; | ||
1267 | |||
1265 | tmp.name[sizeof(tmp.name)-1] = 0; | 1268 | tmp.name[sizeof(tmp.name)-1] = 0; |
1266 | 1269 | ||
1267 | newinfo = xt_alloc_table_info(tmp.size); | 1270 | newinfo = xt_alloc_table_info(tmp.size); |
@@ -1809,6 +1812,9 @@ compat_do_replace(struct net *net, void __user *user, unsigned int len) | |||
1809 | return -ENOMEM; | 1812 | return -ENOMEM; |
1810 | if (tmp.num_counters >= INT_MAX / sizeof(struct xt_counters)) | 1813 | if (tmp.num_counters >= INT_MAX / sizeof(struct xt_counters)) |
1811 | return -ENOMEM; | 1814 | return -ENOMEM; |
1815 | if (tmp.num_counters == 0) | ||
1816 | return -EINVAL; | ||
1817 | |||
1812 | tmp.name[sizeof(tmp.name)-1] = 0; | 1818 | tmp.name[sizeof(tmp.name)-1] = 0; |
1813 | 1819 | ||
1814 | newinfo = xt_alloc_table_info(tmp.size); | 1820 | newinfo = xt_alloc_table_info(tmp.size); |
diff --git a/net/ipv6/netfilter/ip6_tables.c b/net/ipv6/netfilter/ip6_tables.c index 1a732a1d3c8e..62f5b0d0bc9b 100644 --- a/net/ipv6/netfilter/ip6_tables.c +++ b/net/ipv6/netfilter/ip6_tables.c | |||
@@ -1275,6 +1275,9 @@ do_replace(struct net *net, const void __user *user, unsigned int len) | |||
1275 | /* overflow check */ | 1275 | /* overflow check */ |
1276 | if (tmp.num_counters >= INT_MAX / sizeof(struct xt_counters)) | 1276 | if (tmp.num_counters >= INT_MAX / sizeof(struct xt_counters)) |
1277 | return -ENOMEM; | 1277 | return -ENOMEM; |
1278 | if (tmp.num_counters == 0) | ||
1279 | return -EINVAL; | ||
1280 | |||
1278 | tmp.name[sizeof(tmp.name)-1] = 0; | 1281 | tmp.name[sizeof(tmp.name)-1] = 0; |
1279 | 1282 | ||
1280 | newinfo = xt_alloc_table_info(tmp.size); | 1283 | newinfo = xt_alloc_table_info(tmp.size); |
@@ -1822,6 +1825,9 @@ compat_do_replace(struct net *net, void __user *user, unsigned int len) | |||
1822 | return -ENOMEM; | 1825 | return -ENOMEM; |
1823 | if (tmp.num_counters >= INT_MAX / sizeof(struct xt_counters)) | 1826 | if (tmp.num_counters >= INT_MAX / sizeof(struct xt_counters)) |
1824 | return -ENOMEM; | 1827 | return -ENOMEM; |
1828 | if (tmp.num_counters == 0) | ||
1829 | return -EINVAL; | ||
1830 | |||
1825 | tmp.name[sizeof(tmp.name)-1] = 0; | 1831 | tmp.name[sizeof(tmp.name)-1] = 0; |
1826 | 1832 | ||
1827 | newinfo = xt_alloc_table_info(tmp.size); | 1833 | newinfo = xt_alloc_table_info(tmp.size); |