From patchwork Fri Apr 9 14:03:13 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tim Gardner X-Patchwork-Id: 1464390 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=none (no SPF record) smtp.mailfrom=lists.ubuntu.com (client-ip=91.189.94.19; helo=huckleberry.canonical.com; envelope-from=kernel-team-bounces@lists.ubuntu.com; receiver=) Received: from huckleberry.canonical.com (huckleberry.canonical.com [91.189.94.19]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 4FH0Gc5Rrnz9sWk; Sat, 10 Apr 2021 00:03:36 +1000 (AEST) Received: from localhost ([127.0.0.1] helo=huckleberry.canonical.com) by huckleberry.canonical.com with esmtp (Exim 4.86_2) (envelope-from ) id 1lUrjQ-0007Jo-V4; Fri, 09 Apr 2021 14:03:32 +0000 Received: from youngberry.canonical.com ([91.189.89.112]) by huckleberry.canonical.com with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.86_2) (envelope-from ) id 1lUrjP-0007J7-PT for kernel-team@lists.ubuntu.com; Fri, 09 Apr 2021 14:03:31 +0000 Received: from mail-pg1-f198.google.com ([209.85.215.198]) by youngberry.canonical.com with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.86_2) (envelope-from ) id 1lUrjP-0003bp-Bt for kernel-team@lists.ubuntu.com; Fri, 09 Apr 2021 14:03:31 +0000 Received: by mail-pg1-f198.google.com with SMTP id o9so3211677pgm.15 for ; Fri, 09 Apr 2021 07:03:31 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references; bh=zJF6OIkjE2ar9hPSU1xnMLVxFlQ87xkLoyoel38si4o=; b=tQ5ycyTXRCu2L5jJMcZTXnysw0lP7QFq2zauPXuY4oIwxgufbA3CV9gPLeIJ48rEmI 9hdDZ3LwingMeBTMNeYAuNoXlNYEijcwbP1p/PJPusgCMJBf8UFUMdfVOkK4FrvfiLSJ e9X6PqB6s6m0OsEVVPzxMYFKyGzEy/yaKhBVon+7Wgz/whuGRNL/5KrKZpbWljFbVaZO AhLsGbaUA67x975U26ljF43tVNKdrD3od2U8Bo7/Gy+g6i9/soxWG0GcdebTFe4pOFdc M7hPv03tGST0OfyLlbd0fW2IC0lR6xc31LijBWCiVZ+/EP5kw4ze06cC4NBEAnCc4zYS Cakg== X-Gm-Message-State: AOAM530fhQFtHEMOQcxdeOm8YvU0qcKu0E+ScyWA7Cu3Mx0/sCgWmVnN BZ6dN4G8OOKxOjf2Xl0Ynzx5RP1Hob7fOnxVB4t9jL8pGT2eqEtnPnji629O6m9MzvXDQIyLX5M 9/AWl+t3oOLrleaP4CgMN9sVit9ZYOukhJHeoBeM5EQ== X-Received: by 2002:a17:90a:f3d2:: with SMTP id ha18mr14696495pjb.214.1617977009677; Fri, 09 Apr 2021 07:03:29 -0700 (PDT) X-Google-Smtp-Source: ABdhPJxra87Zsa+1HaCLLafjPUgSxNn2/4JGLRz+QwbyE+NTlV3Vqn4yLqZOTFSYpPx6hsmpi0evQw== X-Received: by 2002:a17:90a:f3d2:: with SMTP id ha18mr14696485pjb.214.1617977009476; Fri, 09 Apr 2021 07:03:29 -0700 (PDT) Received: from localhost.localdomain ([69.163.84.166]) by smtp.gmail.com with ESMTPSA id a26sm2584244pff.149.2021.04.09.07.03.28 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 09 Apr 2021 07:03:28 -0700 (PDT) From: Tim Gardner To: kernel-team@lists.ubuntu.com Subject: [PATCH 1/3][Focal] Revert "netfilter: x_tables: Update remaining dereference to RCU" Date: Fri, 9 Apr 2021 08:03:13 -0600 Message-Id: <20210409140321.7279-3-tim.gardner@canonical.com> X-Mailer: git-send-email 2.17.1 In-Reply-To: <20210409140321.7279-1-tim.gardner@canonical.com> References: <20210409140321.7279-1-tim.gardner@canonical.com> X-BeenThere: kernel-team@lists.ubuntu.com X-Mailman-Version: 2.1.20 Precedence: list List-Id: Kernel team discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Errors-To: kernel-team-bounces@lists.ubuntu.com Sender: "kernel-team" From: Mark Tomlinson CVE-2021-29650 This reverts commit 443d6e86f821a165fae3fc3fc13086d27ac140b1. This (and the following) patch basically re-implemented the RCU mechanisms of patch 784544739a25. That patch was replaced because of the performance problems that it created when replacing tables. Now, we have the same issue: the call to synchronize_rcu() makes replacing tables slower by as much as an order of magnitude. Revert these patches and fix the issue in a different way. Signed-off-by: Mark Tomlinson Signed-off-by: Pablo Neira Ayuso (cherry picked from commit abe7034b9a8d57737e80cc16d60ed3666990bdbf) Signed-off-by: Tim Gardner --- net/ipv4/netfilter/arp_tables.c | 2 +- net/ipv4/netfilter/ip_tables.c | 2 +- net/ipv6/netfilter/ip6_tables.c | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/net/ipv4/netfilter/arp_tables.c b/net/ipv4/netfilter/arp_tables.c index 8394afcb2c8f..eaff8c772056 100644 --- a/net/ipv4/netfilter/arp_tables.c +++ b/net/ipv4/netfilter/arp_tables.c @@ -1423,7 +1423,7 @@ static int compat_get_entries(struct net *net, xt_compat_lock(NFPROTO_ARP); t = xt_find_table_lock(net, NFPROTO_ARP, get.name); if (!IS_ERR(t)) { - const struct xt_table_info *private = xt_table_get_private_protected(t); + const struct xt_table_info *private = t->private; struct xt_table_info info; ret = compat_table_info(private, &info); diff --git a/net/ipv4/netfilter/ip_tables.c b/net/ipv4/netfilter/ip_tables.c index 62db61532163..77643a0c4c87 100644 --- a/net/ipv4/netfilter/ip_tables.c +++ b/net/ipv4/netfilter/ip_tables.c @@ -1633,7 +1633,7 @@ compat_get_entries(struct net *net, struct compat_ipt_get_entries __user *uptr, xt_compat_lock(AF_INET); t = xt_find_table_lock(net, AF_INET, get.name); if (!IS_ERR(t)) { - const struct xt_table_info *private = xt_table_get_private_protected(t); + const struct xt_table_info *private = t->private; struct xt_table_info info; ret = compat_table_info(private, &info); if (!ret && get.size == info.size) diff --git a/net/ipv6/netfilter/ip6_tables.c b/net/ipv6/netfilter/ip6_tables.c index 30eb8307582f..5422a1660e3d 100644 --- a/net/ipv6/netfilter/ip6_tables.c +++ b/net/ipv6/netfilter/ip6_tables.c @@ -1642,7 +1642,7 @@ compat_get_entries(struct net *net, struct compat_ip6t_get_entries __user *uptr, xt_compat_lock(AF_INET6); t = xt_find_table_lock(net, AF_INET6, get.name); if (!IS_ERR(t)) { - const struct xt_table_info *private = xt_table_get_private_protected(t); + const struct xt_table_info *private = t->private; struct xt_table_info info; ret = compat_table_info(private, &info); if (!ret && get.size == info.size) From patchwork Fri Apr 9 14:03:14 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tim Gardner X-Patchwork-Id: 1464391 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=none (no SPF record) smtp.mailfrom=lists.ubuntu.com (client-ip=91.189.94.19; helo=huckleberry.canonical.com; envelope-from=kernel-team-bounces@lists.ubuntu.com; receiver=) Received: from huckleberry.canonical.com (huckleberry.canonical.com [91.189.94.19]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 4FH0Gj3SnXz9sW1; Sat, 10 Apr 2021 00:03:41 +1000 (AEST) Received: from localhost ([127.0.0.1] helo=huckleberry.canonical.com) by huckleberry.canonical.com with esmtp (Exim 4.86_2) (envelope-from ) id 1lUrjV-0007Mj-Ep; Fri, 09 Apr 2021 14:03:37 +0000 Received: from youngberry.canonical.com ([91.189.89.112]) by huckleberry.canonical.com with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.86_2) (envelope-from ) id 1lUrjR-0007KF-Q1 for kernel-team@lists.ubuntu.com; Fri, 09 Apr 2021 14:03:33 +0000 Received: from mail-pl1-f197.google.com ([209.85.214.197]) by youngberry.canonical.com with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.86_2) (envelope-from ) id 1lUrjQ-0003c8-Rj for kernel-team@lists.ubuntu.com; Fri, 09 Apr 2021 14:03:33 +0000 Received: by mail-pl1-f197.google.com with SMTP id s23so2278392plp.1 for ; Fri, 09 Apr 2021 07:03:32 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references; bh=lFMwC7TlqlF5jvofjTV6G5busbm72YPjPjzxHDt2fiA=; b=LiquLmxi9PvGvMG+O0He8PMSQueIlP+eglanf9aRgmecFAI/iQxU5Do3Ac+Hp3kyxy gyOsZgsMSNn+4MNsBCCx+1z/+jXtuDkYM3jM0jEmsdI1UBPZ3HEOTeP0keqcXgXvKcNq r9j9CSfhsUzVpDYCdaJ71q3RDP19GoBCAYGkTMXEixWqEQaiNBtE5s2pNK2MwKk+2DVX qi5NUcMuC0vPGBxrIay4BFNGNFiaphTgZ80Ebam1ergKspostpRSZ6qU8UwMGB2Ot7Dc 0J05da+EAIsMLkMCFGt54cmYwsS6Nc2kIFyKu0G8yDksox7wAbHnFi3Tzr30DnIovvjs KAiA== X-Gm-Message-State: AOAM532b4W3yajZamSUl3U4VCITWLbccfqcLWm8XcbPLliyJ7Z7BPc7R cys/B+Vg0WLa7HzI8pUBDHYq0RYUXpouSt6V9jFOzEciOhHPlSRbpJtEqmZu0l8JoB3/l2MUfCx 22dpPta8kmRfL4cx93gNV5nluV3PeHT1zxsMhfA11Lg== X-Received: by 2002:a17:90a:5306:: with SMTP id x6mr9152700pjh.39.1617977010958; Fri, 09 Apr 2021 07:03:30 -0700 (PDT) X-Google-Smtp-Source: ABdhPJyS74zaLigB3Nnfo4xcQCBnNc8ShmofBJc1Gv0n9m/hJQXHQEcRtRxjvG8BGKWDwz8mNzGyBA== X-Received: by 2002:a17:90a:5306:: with SMTP id x6mr9152679pjh.39.1617977010673; Fri, 09 Apr 2021 07:03:30 -0700 (PDT) Received: from localhost.localdomain ([69.163.84.166]) by smtp.gmail.com with ESMTPSA id a26sm2584244pff.149.2021.04.09.07.03.29 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 09 Apr 2021 07:03:30 -0700 (PDT) From: Tim Gardner To: kernel-team@lists.ubuntu.com Subject: [PATCH 2/3][Focal] Revert "netfilter: x_tables: Switch synchronization to RCU" Date: Fri, 9 Apr 2021 08:03:14 -0600 Message-Id: <20210409140321.7279-4-tim.gardner@canonical.com> X-Mailer: git-send-email 2.17.1 In-Reply-To: <20210409140321.7279-1-tim.gardner@canonical.com> References: <20210409140321.7279-1-tim.gardner@canonical.com> X-BeenThere: kernel-team@lists.ubuntu.com X-Mailman-Version: 2.1.20 Precedence: list List-Id: Kernel team discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Errors-To: kernel-team-bounces@lists.ubuntu.com Sender: "kernel-team" From: Mark Tomlinson CVE-2021-29650 This reverts commit cc00bcaa589914096edef7fb87ca5cee4a166b5c. This (and the preceding) patch basically re-implemented the RCU mechanisms of patch 784544739a25. That patch was replaced because of the performance problems that it created when replacing tables. Now, we have the same issue: the call to synchronize_rcu() makes replacing tables slower by as much as an order of magnitude. Prior to using RCU a script calling "iptables" approx. 200 times was taking 1.16s. With RCU this increased to 11.59s. Revert these patches and fix the issue in a different way. Signed-off-by: Mark Tomlinson Signed-off-by: Pablo Neira Ayuso (cherry picked from commit d3d40f237480abf3268956daf18cdc56edd32834) Signed-off-by: Tim Gardner --- include/linux/netfilter/x_tables.h | 5 +-- net/ipv4/netfilter/arp_tables.c | 14 ++++----- net/ipv4/netfilter/ip_tables.c | 14 ++++----- net/ipv6/netfilter/ip6_tables.c | 14 ++++----- net/netfilter/x_tables.c | 49 +++++++++++++++++++++--------- 5 files changed, 56 insertions(+), 40 deletions(-) diff --git a/include/linux/netfilter/x_tables.h b/include/linux/netfilter/x_tables.h index f5c21b7d2974..1b261c51b3a3 100644 --- a/include/linux/netfilter/x_tables.h +++ b/include/linux/netfilter/x_tables.h @@ -227,7 +227,7 @@ struct xt_table { unsigned int valid_hooks; /* Man behind the curtain... */ - struct xt_table_info __rcu *private; + struct xt_table_info *private; /* Set this to THIS_MODULE if you are a module, otherwise NULL */ struct module *me; @@ -448,9 +448,6 @@ xt_get_per_cpu_counter(struct xt_counters *cnt, unsigned int cpu) struct nf_hook_ops *xt_hook_ops_alloc(const struct xt_table *, nf_hookfn *); -struct xt_table_info -*xt_table_get_private_protected(const struct xt_table *table); - #ifdef CONFIG_COMPAT #include diff --git a/net/ipv4/netfilter/arp_tables.c b/net/ipv4/netfilter/arp_tables.c index eaff8c772056..557c295f2d9a 100644 --- a/net/ipv4/netfilter/arp_tables.c +++ b/net/ipv4/netfilter/arp_tables.c @@ -203,7 +203,7 @@ unsigned int arpt_do_table(struct sk_buff *skb, local_bh_disable(); addend = xt_write_recseq_begin(); - private = rcu_access_pointer(table->private); + private = READ_ONCE(table->private); /* Address dependency. */ cpu = smp_processor_id(); table_base = private->entries; jumpstack = (struct arpt_entry **)private->jumpstack[cpu]; @@ -666,7 +666,7 @@ static struct xt_counters *alloc_counters(const struct xt_table *table) { unsigned int countersize; struct xt_counters *counters; - const struct xt_table_info *private = xt_table_get_private_protected(table); + const struct xt_table_info *private = table->private; /* We need atomic snapshot of counters: rest doesn't change * (other than comefrom, which userspace doesn't care @@ -690,7 +690,7 @@ static int copy_entries_to_user(unsigned int total_size, unsigned int off, num; const struct arpt_entry *e; struct xt_counters *counters; - struct xt_table_info *private = xt_table_get_private_protected(table); + struct xt_table_info *private = table->private; int ret = 0; void *loc_cpu_entry; @@ -825,7 +825,7 @@ static int get_info(struct net *net, void __user *user, t = xt_request_find_table_lock(net, NFPROTO_ARP, name); if (!IS_ERR(t)) { struct arpt_getinfo info; - const struct xt_table_info *private = xt_table_get_private_protected(t); + const struct xt_table_info *private = t->private; #ifdef CONFIG_COMPAT struct xt_table_info tmp; @@ -878,7 +878,7 @@ static int get_entries(struct net *net, struct arpt_get_entries __user *uptr, t = xt_find_table_lock(net, NFPROTO_ARP, get.name); if (!IS_ERR(t)) { - const struct xt_table_info *private = xt_table_get_private_protected(t); + const struct xt_table_info *private = t->private; if (get.size == private->size) ret = copy_entries_to_user(private->size, @@ -1037,7 +1037,7 @@ static int do_add_counters(struct net *net, const void __user *user, } local_bh_disable(); - private = xt_table_get_private_protected(t); + private = t->private; if (private->number != tmp.num_counters) { ret = -EINVAL; goto unlock_up_free; @@ -1374,7 +1374,7 @@ static int compat_copy_entries_to_user(unsigned int total_size, void __user *userptr) { struct xt_counters *counters; - const struct xt_table_info *private = xt_table_get_private_protected(table); + const struct xt_table_info *private = table->private; void __user *pos; unsigned int size; int ret = 0; diff --git a/net/ipv4/netfilter/ip_tables.c b/net/ipv4/netfilter/ip_tables.c index 77643a0c4c87..3a14153b8d07 100644 --- a/net/ipv4/netfilter/ip_tables.c +++ b/net/ipv4/netfilter/ip_tables.c @@ -258,7 +258,7 @@ ipt_do_table(struct sk_buff *skb, WARN_ON(!(table->valid_hooks & (1 << hook))); local_bh_disable(); addend = xt_write_recseq_begin(); - private = rcu_access_pointer(table->private); + private = READ_ONCE(table->private); /* Address dependency. */ cpu = smp_processor_id(); table_base = private->entries; jumpstack = (struct ipt_entry **)private->jumpstack[cpu]; @@ -808,7 +808,7 @@ static struct xt_counters *alloc_counters(const struct xt_table *table) { unsigned int countersize; struct xt_counters *counters; - const struct xt_table_info *private = xt_table_get_private_protected(table); + const struct xt_table_info *private = table->private; /* We need atomic snapshot of counters: rest doesn't change (other than comefrom, which userspace doesn't care @@ -832,7 +832,7 @@ copy_entries_to_user(unsigned int total_size, unsigned int off, num; const struct ipt_entry *e; struct xt_counters *counters; - const struct xt_table_info *private = xt_table_get_private_protected(table); + const struct xt_table_info *private = table->private; int ret = 0; const void *loc_cpu_entry; @@ -982,7 +982,7 @@ static int get_info(struct net *net, void __user *user, t = xt_request_find_table_lock(net, AF_INET, name); if (!IS_ERR(t)) { struct ipt_getinfo info; - const struct xt_table_info *private = xt_table_get_private_protected(t); + const struct xt_table_info *private = t->private; #ifdef CONFIG_COMPAT struct xt_table_info tmp; @@ -1036,7 +1036,7 @@ get_entries(struct net *net, struct ipt_get_entries __user *uptr, t = xt_find_table_lock(net, AF_INET, get.name); if (!IS_ERR(t)) { - const struct xt_table_info *private = xt_table_get_private_protected(t); + const struct xt_table_info *private = t->private; if (get.size == private->size) ret = copy_entries_to_user(private->size, t, uptr->entrytable); @@ -1192,7 +1192,7 @@ do_add_counters(struct net *net, const void __user *user, } local_bh_disable(); - private = xt_table_get_private_protected(t); + private = t->private; if (private->number != tmp.num_counters) { ret = -EINVAL; goto unlock_up_free; @@ -1587,7 +1587,7 @@ compat_copy_entries_to_user(unsigned int total_size, struct xt_table *table, void __user *userptr) { struct xt_counters *counters; - const struct xt_table_info *private = xt_table_get_private_protected(table); + const struct xt_table_info *private = table->private; void __user *pos; unsigned int size; int ret = 0; diff --git a/net/ipv6/netfilter/ip6_tables.c b/net/ipv6/netfilter/ip6_tables.c index 5422a1660e3d..d7c16a30156f 100644 --- a/net/ipv6/netfilter/ip6_tables.c +++ b/net/ipv6/netfilter/ip6_tables.c @@ -280,7 +280,7 @@ ip6t_do_table(struct sk_buff *skb, local_bh_disable(); addend = xt_write_recseq_begin(); - private = rcu_access_pointer(table->private); + private = READ_ONCE(table->private); /* Address dependency. */ cpu = smp_processor_id(); table_base = private->entries; jumpstack = (struct ip6t_entry **)private->jumpstack[cpu]; @@ -824,7 +824,7 @@ static struct xt_counters *alloc_counters(const struct xt_table *table) { unsigned int countersize; struct xt_counters *counters; - const struct xt_table_info *private = xt_table_get_private_protected(table); + const struct xt_table_info *private = table->private; /* We need atomic snapshot of counters: rest doesn't change (other than comefrom, which userspace doesn't care @@ -848,7 +848,7 @@ copy_entries_to_user(unsigned int total_size, unsigned int off, num; const struct ip6t_entry *e; struct xt_counters *counters; - const struct xt_table_info *private = xt_table_get_private_protected(table); + const struct xt_table_info *private = table->private; int ret = 0; const void *loc_cpu_entry; @@ -998,7 +998,7 @@ static int get_info(struct net *net, void __user *user, t = xt_request_find_table_lock(net, AF_INET6, name); if (!IS_ERR(t)) { struct ip6t_getinfo info; - const struct xt_table_info *private = xt_table_get_private_protected(t); + const struct xt_table_info *private = t->private; #ifdef CONFIG_COMPAT struct xt_table_info tmp; @@ -1053,7 +1053,7 @@ get_entries(struct net *net, struct ip6t_get_entries __user *uptr, t = xt_find_table_lock(net, AF_INET6, get.name); if (!IS_ERR(t)) { - struct xt_table_info *private = xt_table_get_private_protected(t); + struct xt_table_info *private = t->private; if (get.size == private->size) ret = copy_entries_to_user(private->size, t, uptr->entrytable); @@ -1208,7 +1208,7 @@ do_add_counters(struct net *net, const void __user *user, unsigned int len, } local_bh_disable(); - private = xt_table_get_private_protected(t); + private = t->private; if (private->number != tmp.num_counters) { ret = -EINVAL; goto unlock_up_free; @@ -1596,7 +1596,7 @@ compat_copy_entries_to_user(unsigned int total_size, struct xt_table *table, void __user *userptr) { struct xt_counters *counters; - const struct xt_table_info *private = xt_table_get_private_protected(table); + const struct xt_table_info *private = table->private; void __user *pos; unsigned int size; int ret = 0; diff --git a/net/netfilter/x_tables.c b/net/netfilter/x_tables.c index 8b60fc04c67c..ef6d51a3798b 100644 --- a/net/netfilter/x_tables.c +++ b/net/netfilter/x_tables.c @@ -1351,14 +1351,6 @@ struct xt_counters *xt_counters_alloc(unsigned int counters) } EXPORT_SYMBOL(xt_counters_alloc); -struct xt_table_info -*xt_table_get_private_protected(const struct xt_table *table) -{ - return rcu_dereference_protected(table->private, - mutex_is_locked(&xt[table->af].mutex)); -} -EXPORT_SYMBOL(xt_table_get_private_protected); - struct xt_table_info * xt_replace_table(struct xt_table *table, unsigned int num_counters, @@ -1366,6 +1358,7 @@ xt_replace_table(struct xt_table *table, int *error) { struct xt_table_info *private; + unsigned int cpu; int ret; ret = xt_jumpstack_alloc(newinfo); @@ -1375,20 +1368,47 @@ xt_replace_table(struct xt_table *table, } /* Do the substitution. */ - private = xt_table_get_private_protected(table); + local_bh_disable(); + private = table->private; /* Check inside lock: is the old number correct? */ if (num_counters != private->number) { pr_debug("num_counters != table->private->number (%u/%u)\n", num_counters, private->number); + local_bh_enable(); *error = -EAGAIN; return NULL; } newinfo->initial_entries = private->initial_entries; + /* + * Ensure contents of newinfo are visible before assigning to + * private. + */ + smp_wmb(); + table->private = newinfo; + + /* make sure all cpus see new ->private value */ + smp_wmb(); - rcu_assign_pointer(table->private, newinfo); - synchronize_rcu(); + /* + * Even though table entries have now been swapped, other CPU's + * may still be using the old entries... + */ + local_bh_enable(); + + /* ... so wait for even xt_recseq on all cpus */ + for_each_possible_cpu(cpu) { + seqcount_t *s = &per_cpu(xt_recseq, cpu); + u32 seq = raw_read_seqcount(s); + + if (seq & 1) { + do { + cond_resched(); + cpu_relax(); + } while (seq == raw_read_seqcount(s)); + } + } #ifdef CONFIG_AUDIT if (audit_enabled) { @@ -1429,12 +1449,12 @@ struct xt_table *xt_register_table(struct net *net, } /* Simplifies replace_table code. */ - rcu_assign_pointer(table->private, bootstrap); + table->private = bootstrap; if (!xt_replace_table(table, 0, newinfo, &ret)) goto unlock; - private = xt_table_get_private_protected(table); + private = table->private; pr_debug("table->private->number = %u\n", private->number); /* save number of initial entries */ @@ -1457,8 +1477,7 @@ void *xt_unregister_table(struct xt_table *table) struct xt_table_info *private; mutex_lock(&xt[table->af].mutex); - private = xt_table_get_private_protected(table); - RCU_INIT_POINTER(table->private, NULL); + private = table->private; list_del(&table->list); mutex_unlock(&xt[table->af].mutex); kfree(table); From patchwork Fri Apr 9 14:03:15 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tim Gardner X-Patchwork-Id: 1464392 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=none (no SPF record) smtp.mailfrom=lists.ubuntu.com (client-ip=91.189.94.19; helo=huckleberry.canonical.com; envelope-from=kernel-team-bounces@lists.ubuntu.com; receiver=) Received: from huckleberry.canonical.com (huckleberry.canonical.com [91.189.94.19]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 4FH0Gk2hQkz9sWD; Sat, 10 Apr 2021 00:03:42 +1000 (AEST) Received: from localhost ([127.0.0.1] helo=huckleberry.canonical.com) by huckleberry.canonical.com with esmtp (Exim 4.86_2) (envelope-from ) id 1lUrjW-0007NN-5Z; Fri, 09 Apr 2021 14:03:38 +0000 Received: from youngberry.canonical.com ([91.189.89.112]) by huckleberry.canonical.com with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.86_2) (envelope-from ) id 1lUrjS-0007Kv-Gd for kernel-team@lists.ubuntu.com; Fri, 09 Apr 2021 14:03:34 +0000 Received: from mail-pg1-f198.google.com ([209.85.215.198]) by youngberry.canonical.com with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.86_2) (envelope-from ) id 1lUrjS-0003cB-3w for kernel-team@lists.ubuntu.com; Fri, 09 Apr 2021 14:03:34 +0000 Received: by mail-pg1-f198.google.com with SMTP id n23so3225027pgl.2 for ; Fri, 09 Apr 2021 07:03:34 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references; bh=dDyO3+bXcn5sAs14A3dOXH7A6FBK6tYzWkLzO3Nv3YU=; b=ipkw5Ofx3QIaHBaV3Qt0J4CwvkyX12Cam6Yq6v8W5T+gO6/0vn5mBnh/+q4THgmXZ7 AdrdjH2gYX/19FTOW10uVnMKi1VOXikF7560zVws72R7YWzsdKYgW7IOg0KtMLtIwpHy NPAN4RqFCOOcQhkXVR4flq0tNala3jztusWqLGyP0bQI7oLePme0lcNElpE7yuQZStPj CYneu21Gp0sLD2k7aRUx72T8TRTqdCxUSawp7eofu1S84mtNg83zNs1rjsDwNbtGyiMV HNEVHgGYM3rKDxEZwYsnS8t+H+XKxSBByfUidpcCISEDwoc4s8j4zN3qRczEquSJpOzt Fgyg== X-Gm-Message-State: AOAM5333QD0K/NLd4vjGa2ThwpgK3jD/Fc04dLK6i6F29zmZeFRcDhnT 2e9EdYIx6SjdvKSIdwwNWYARtYfbN5Siw0Bwqmv0LnvLKeRRUah+g+yLRx9Rnt41cdSdTDa6U7m OaTXI+LMWg1tZZYp11p7LocZHKQOiTA4qPCS9S3di9w== X-Received: by 2002:aa7:8f10:0:b029:247:219e:e78 with SMTP id x16-20020aa78f100000b0290247219e0e78mr2822730pfr.15.1617977012350; Fri, 09 Apr 2021 07:03:32 -0700 (PDT) X-Google-Smtp-Source: ABdhPJxrIIlgjdDdQ1hzo5LoyZt48J2tpiYh/98q7T/rJ9dK410JdymmbXDjqDfVwSPbejOV30etBA== X-Received: by 2002:aa7:8f10:0:b029:247:219e:e78 with SMTP id x16-20020aa78f100000b0290247219e0e78mr2822715pfr.15.1617977012153; Fri, 09 Apr 2021 07:03:32 -0700 (PDT) Received: from localhost.localdomain ([69.163.84.166]) by smtp.gmail.com with ESMTPSA id a26sm2584244pff.149.2021.04.09.07.03.30 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 09 Apr 2021 07:03:31 -0700 (PDT) From: Tim Gardner To: kernel-team@lists.ubuntu.com Subject: [PATCH 3/3][Focal] netfilter: x_tables: Use correct memory barriers. Date: Fri, 9 Apr 2021 08:03:15 -0600 Message-Id: <20210409140321.7279-5-tim.gardner@canonical.com> X-Mailer: git-send-email 2.17.1 In-Reply-To: <20210409140321.7279-1-tim.gardner@canonical.com> References: <20210409140321.7279-1-tim.gardner@canonical.com> X-BeenThere: kernel-team@lists.ubuntu.com X-Mailman-Version: 2.1.20 Precedence: list List-Id: Kernel team discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Errors-To: kernel-team-bounces@lists.ubuntu.com Sender: "kernel-team" From: Mark Tomlinson CVE-2021-29650 When a new table value was assigned, it was followed by a write memory barrier. This ensured that all writes before this point would complete before any writes after this point. However, to determine whether the rules are unused, the sequence counter is read. To ensure that all writes have been done before these reads, a full memory barrier is needed, not just a write memory barrier. The same argument applies when incrementing the counter, before the rules are read. Changing to using smp_mb() instead of smp_wmb() fixes the kernel panic reported in cc00bcaa5899 (which is still present), while still maintaining the same speed of replacing tables. The smb_mb() barriers potentially slow the packet path, however testing has shown no measurable change in performance on a 4-core MIPS64 platform. Fixes: 7f5c6d4f665b ("netfilter: get rid of atomic ops in fast path") Signed-off-by: Mark Tomlinson Signed-off-by: Pablo Neira Ayuso (cherry picked from commit 175e476b8cdf2a4de7432583b49c871345e4f8a1) Signed-off-by: Tim Gardner --- include/linux/netfilter/x_tables.h | 2 +- net/netfilter/x_tables.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/include/linux/netfilter/x_tables.h b/include/linux/netfilter/x_tables.h index 1b261c51b3a3..04e7f5630509 100644 --- a/include/linux/netfilter/x_tables.h +++ b/include/linux/netfilter/x_tables.h @@ -376,7 +376,7 @@ static inline unsigned int xt_write_recseq_begin(void) * since addend is most likely 1 */ __this_cpu_add(xt_recseq.sequence, addend); - smp_wmb(); + smp_mb(); return addend; } diff --git a/net/netfilter/x_tables.c b/net/netfilter/x_tables.c index ef6d51a3798b..5c35d64d1f34 100644 --- a/net/netfilter/x_tables.c +++ b/net/netfilter/x_tables.c @@ -1389,7 +1389,7 @@ xt_replace_table(struct xt_table *table, table->private = newinfo; /* make sure all cpus see new ->private value */ - smp_wmb(); + smp_mb(); /* * Even though table entries have now been swapped, other CPU's