netfilter: x_tables: avoid leaking percpu counter pointers

The native and compat get-entries paths copy the fixed rule entry header
from the kernelized rule blob to userspace before overwriting the entry's
counter fields with a sanitized counter snapshot.

On SMP kernels, entry->counters.pcnt contains the percpu allocation
address used by x_tables rule counters. A caller can provide a userspace
buffer that faults during the initial fixed-header copy after pcnt has
been copied but before the later sanitized counter copy runs. The syscall
then returns -EFAULT while leaving the raw percpu pointer in userspace.

Copy only the fixed entry prefix before counters from the kernelized rule
blob, then copy the sanitized counter snapshot into the counter field.
Apply this ordering to the IPv4, IPv6, and ARP native and compat
get-entries implementations so a fault cannot expose the internal percpu
counter pointer.

Fixes: 71ae0dff02d7 ("netfilter: xtables: use percpu rule counters")
Signed-off-by: Kyle Zeng <kylebot@openai.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>

diff --git a/net/ipv4/netfilter/arp_tables.c b/net/ipv4/netfilter/arp_tables.c
index ad2259678c7854359a7c909aa82aa93763a932fc..0ea513bf77fb6a46ba8bb7db247dd692863ca0ec 100644 (file)
--- a/net/ipv4/netfilter/arp_tables.c
+++ b/net/ipv4/netfilter/arp_tables.c
@@ -702,14 +702,12 @@ static int copy_entries_to_user(unsigned int total_size,
                const struct xt_entry_target *t;
 
                e = loc_cpu_entry + off;
-               if (copy_to_user(userptr + off, e, sizeof(*e))) {
-                       ret = -EFAULT;
-                       goto free_counters;
-               }
-               if (copy_to_user(userptr + off
+               if (copy_to_user(userptr + off, e,
+                                offsetof(struct arpt_entry, counters)) ||
+                   copy_to_user(userptr + off
                                 + offsetof(struct arpt_entry, counters),
                                 &counters[num],
-                                sizeof(counters[num])) != 0) {
+                                sizeof(counters[num]))) {
                        ret = -EFAULT;
                        goto free_counters;
                }
@@ -1327,9 +1325,8 @@ static int compat_copy_entry_to_user(struct arpt_entry *e, void __user **dstptr,
 
        origsize = *size;
        ce = *dstptr;
-       if (copy_to_user(ce, e, sizeof(struct arpt_entry)) != 0 ||
-           copy_to_user(&ce->counters, &counters[i],
-           sizeof(counters[i])) != 0)
+       if (copy_to_user(ce, e, offsetof(struct compat_arpt_entry, counters)) ||
+           copy_to_user(&ce->counters, &counters[i], sizeof(counters[i])))
                return -EFAULT;
 
        *dstptr += sizeof(struct compat_arpt_entry);

diff --git a/net/ipv4/netfilter/ip_tables.c b/net/ipv4/netfilter/ip_tables.c
index 5cbdb0815857f402df4bb57bb9dddc327ff01841..ca8ff0ae6cdb9f1bb7c1f305d820f918b4118bf7 100644 (file)
--- a/net/ipv4/netfilter/ip_tables.c
+++ b/net/ipv4/netfilter/ip_tables.c
@@ -832,14 +832,12 @@ copy_entries_to_user(unsigned int total_size,
                const struct xt_entry_target *t;
 
                e = loc_cpu_entry + off;
-               if (copy_to_user(userptr + off, e, sizeof(*e))) {
-                       ret = -EFAULT;
-                       goto free_counters;
-               }
-               if (copy_to_user(userptr + off
+               if (copy_to_user(userptr + off, e,
+                                offsetof(struct ipt_entry, counters)) ||
+                   copy_to_user(userptr + off
                                 + offsetof(struct ipt_entry, counters),
                                 &counters[num],
-                                sizeof(counters[num])) != 0) {
+                                sizeof(counters[num]))) {
                        ret = -EFAULT;
                        goto free_counters;
                }
@@ -1228,9 +1226,8 @@ compat_copy_entry_to_user(struct ipt_entry *e, void __user **dstptr,
 
        origsize = *size;
        ce = *dstptr;
-       if (copy_to_user(ce, e, sizeof(struct ipt_entry)) != 0 ||
-           copy_to_user(&ce->counters, &counters[i],
-           sizeof(counters[i])) != 0)
+       if (copy_to_user(ce, e, offsetof(struct compat_ipt_entry, counters)) ||
+           copy_to_user(&ce->counters, &counters[i], sizeof(counters[i])))
                return -EFAULT;
 
        *dstptr += sizeof(struct compat_ipt_entry);

diff --git a/net/ipv6/netfilter/ip6_tables.c b/net/ipv6/netfilter/ip6_tables.c
index 9d9c3763f2f5e90a3d6ed50efbaed5e5865911ae..e34d5ba1460ca7c1c3aa1c7bb601a722f922829a 100644 (file)
--- a/net/ipv6/netfilter/ip6_tables.c
+++ b/net/ipv6/netfilter/ip6_tables.c
@@ -848,14 +848,12 @@ copy_entries_to_user(unsigned int total_size,
                const struct xt_entry_target *t;
 
                e = loc_cpu_entry + off;
-               if (copy_to_user(userptr + off, e, sizeof(*e))) {
-                       ret = -EFAULT;
-                       goto free_counters;
-               }
-               if (copy_to_user(userptr + off
+               if (copy_to_user(userptr + off, e,
+                                offsetof(struct ip6t_entry, counters)) ||
+                   copy_to_user(userptr + off
                                 + offsetof(struct ip6t_entry, counters),
                                 &counters[num],
-                                sizeof(counters[num])) != 0) {
+                                sizeof(counters[num]))) {
                        ret = -EFAULT;
                        goto free_counters;
                }
@@ -1244,9 +1242,8 @@ compat_copy_entry_to_user(struct ip6t_entry *e, void __user **dstptr,
 
        origsize = *size;
        ce = *dstptr;
-       if (copy_to_user(ce, e, sizeof(struct ip6t_entry)) != 0 ||
-           copy_to_user(&ce->counters, &counters[i],
-           sizeof(counters[i])) != 0)
+       if (copy_to_user(ce, e, offsetof(struct compat_ip6t_entry, counters)) ||
+           copy_to_user(&ce->counters, &counters[i], sizeof(counters[i])))
                return -EFAULT;
 
        *dstptr += sizeof(struct compat_ip6t_entry);