LVS
lvs-devel
Google
 
Web LinuxVirtualServer.org

[PATCH 2/2] ipvs: move old_secure_tcp into struct netns_ipvs

To: Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx>
Subject: [PATCH 2/2] ipvs: move old_secure_tcp into struct netns_ipvs
Cc: lvs-devel@xxxxxxxxxxxxxxx, netdev@xxxxxxxxxxxxxxx, netfilter-devel@xxxxxxxxxxxxxxx, Wensong Zhang <wensong@xxxxxxxxxxxx>, Julian Anastasov <ja@xxxxxx>, Eric Dumazet <edumazet@xxxxxxxxxx>, syzbot <syzkaller@xxxxxxxxxxxxxxxx>, Simon Horman <horms@xxxxxxxxxxxx>
From: Simon Horman <horms@xxxxxxxxxxxx>
Date: Fri, 25 Oct 2019 13:12:05 +0200
From: Eric Dumazet <edumazet@xxxxxxxxxx>

syzbot reported the following issue :

BUG: KCSAN: data-race in update_defense_level / update_defense_level

read to 0xffffffff861a6260 of 4 bytes by task 3006 on cpu 1:
 update_defense_level+0x621/0xb30 net/netfilter/ipvs/ip_vs_ctl.c:177
 defense_work_handler+0x3d/0xd0 net/netfilter/ipvs/ip_vs_ctl.c:225
 process_one_work+0x3d4/0x890 kernel/workqueue.c:2269
 worker_thread+0xa0/0x800 kernel/workqueue.c:2415
 kthread+0x1d4/0x200 drivers/block/aoe/aoecmd.c:1253
 ret_from_fork+0x1f/0x30 arch/x86/entry/entry_64.S:352

write to 0xffffffff861a6260 of 4 bytes by task 7333 on cpu 0:
 update_defense_level+0xa62/0xb30 net/netfilter/ipvs/ip_vs_ctl.c:205
 defense_work_handler+0x3d/0xd0 net/netfilter/ipvs/ip_vs_ctl.c:225
 process_one_work+0x3d4/0x890 kernel/workqueue.c:2269
 worker_thread+0xa0/0x800 kernel/workqueue.c:2415
 kthread+0x1d4/0x200 drivers/block/aoe/aoecmd.c:1253
 ret_from_fork+0x1f/0x30 arch/x86/entry/entry_64.S:352

Reported by Kernel Concurrency Sanitizer on:
CPU: 0 PID: 7333 Comm: kworker/0:5 Not tainted 5.4.0-rc3+ #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 
01/01/2011
Workqueue: events defense_work_handler

Indeed, old_secure_tcp is currently a static variable, while it
needs to be a per netns variable.

Fixes: a0840e2e165a ("IPVS: netns, ip_vs_ctl local vars moved to ipvs struct.")
Signed-off-by: Eric Dumazet <edumazet@xxxxxxxxxx>
Reported-by: syzbot <syzkaller@xxxxxxxxxxxxxxxx>
Signed-off-by: Simon Horman <horms@xxxxxxxxxxxx>
---
 include/net/ip_vs.h            |  1 +
 net/netfilter/ipvs/ip_vs_ctl.c | 15 +++++++--------
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/include/net/ip_vs.h b/include/net/ip_vs.h
index 3759167f91f5..078887c8c586 100644
--- a/include/net/ip_vs.h
+++ b/include/net/ip_vs.h
@@ -889,6 +889,7 @@ struct netns_ipvs {
        struct delayed_work     defense_work;   /* Work handler */
        int                     drop_rate;
        int                     drop_counter;
+       int                     old_secure_tcp;
        atomic_t                dropentry;
        /* locks in ctl.c */
        spinlock_t              dropentry_lock;  /* drop entry handling */
diff --git a/net/netfilter/ipvs/ip_vs_ctl.c b/net/netfilter/ipvs/ip_vs_ctl.c
index c8f81dd15c83..3cccc88ef817 100644
--- a/net/netfilter/ipvs/ip_vs_ctl.c
+++ b/net/netfilter/ipvs/ip_vs_ctl.c
@@ -93,7 +93,6 @@ static bool __ip_vs_addr_is_local_v6(struct net *net,
 static void update_defense_level(struct netns_ipvs *ipvs)
 {
        struct sysinfo i;
-       static int old_secure_tcp = 0;
        int availmem;
        int nomem;
        int to_change = -1;
@@ -174,35 +173,35 @@ static void update_defense_level(struct netns_ipvs *ipvs)
        spin_lock(&ipvs->securetcp_lock);
        switch (ipvs->sysctl_secure_tcp) {
        case 0:
-               if (old_secure_tcp >= 2)
+               if (ipvs->old_secure_tcp >= 2)
                        to_change = 0;
                break;
        case 1:
                if (nomem) {
-                       if (old_secure_tcp < 2)
+                       if (ipvs->old_secure_tcp < 2)
                                to_change = 1;
                        ipvs->sysctl_secure_tcp = 2;
                } else {
-                       if (old_secure_tcp >= 2)
+                       if (ipvs->old_secure_tcp >= 2)
                                to_change = 0;
                }
                break;
        case 2:
                if (nomem) {
-                       if (old_secure_tcp < 2)
+                       if (ipvs->old_secure_tcp < 2)
                                to_change = 1;
                } else {
-                       if (old_secure_tcp >= 2)
+                       if (ipvs->old_secure_tcp >= 2)
                                to_change = 0;
                        ipvs->sysctl_secure_tcp = 1;
                }
                break;
        case 3:
-               if (old_secure_tcp < 2)
+               if (ipvs->old_secure_tcp < 2)
                        to_change = 1;
                break;
        }
-       old_secure_tcp = ipvs->sysctl_secure_tcp;
+       ipvs->old_secure_tcp = ipvs->sysctl_secure_tcp;
        if (to_change >= 0)
                ip_vs_protocol_timeout_change(ipvs,
                                              ipvs->sysctl_secure_tcp > 1);
-- 
2.20.1


<Prev in Thread] Current Thread [Next in Thread>