net: annotate data-races around sk->sk_lingertime
authorEric Dumazet <edumazet@google.com>
Sat, 19 Aug 2023 04:06:46 +0000 (04:06 +0000)
committerDavid S. Miller <davem@davemloft.net>
Mon, 21 Aug 2023 06:41:57 +0000 (07:41 +0100)
sk_getsockopt() runs locklessly. This means sk->sk_lingertime
can be read while other threads are changing its value.

Other reads also happen without socket lock being held,
and must be annotated.

Remove preprocessor logic using BITS_PER_LONG, compilers
are smart enough to figure this by themselves.

v2: fixed a clang W=1 (-Wtautological-constant-out-of-range-compare) warning
    (Jakub)

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Eric Dumazet <edumazet@google.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
net/bluetooth/iso.c
net/bluetooth/sco.c
net/core/sock.c
net/sched/em_meta.c
net/smc/af_smc.c

index 6b66d6a..3c03e49 100644 (file)
@@ -1475,7 +1475,7 @@ static int iso_sock_release(struct socket *sock)
 
        iso_sock_close(sk);
 
-       if (sock_flag(sk, SOCK_LINGER) && sk->sk_lingertime &&
+       if (sock_flag(sk, SOCK_LINGER) && READ_ONCE(sk->sk_lingertime) &&
            !(current->flags & PF_EXITING)) {
                lock_sock(sk);
                err = bt_sock_wait_state(sk, BT_CLOSED, sk->sk_lingertime);
index 50ad593..c736186 100644 (file)
@@ -1245,7 +1245,7 @@ static int sco_sock_release(struct socket *sock)
 
        sco_sock_close(sk);
 
-       if (sock_flag(sk, SOCK_LINGER) && sk->sk_lingertime &&
+       if (sock_flag(sk, SOCK_LINGER) && READ_ONCE(sk->sk_lingertime) &&
            !(current->flags & PF_EXITING)) {
                lock_sock(sk);
                err = bt_sock_wait_state(sk, BT_CLOSED, sk->sk_lingertime);
index a072295..666a17c 100644 (file)
@@ -797,7 +797,7 @@ EXPORT_SYMBOL(sock_set_reuseport);
 void sock_no_linger(struct sock *sk)
 {
        lock_sock(sk);
-       sk->sk_lingertime = 0;
+       WRITE_ONCE(sk->sk_lingertime, 0);
        sock_set_flag(sk, SOCK_LINGER);
        release_sock(sk);
 }
@@ -1230,15 +1230,15 @@ set_sndbuf:
                        ret = -EFAULT;
                        break;
                }
-               if (!ling.l_onoff)
+               if (!ling.l_onoff) {
                        sock_reset_flag(sk, SOCK_LINGER);
-               else {
-#if (BITS_PER_LONG == 32)
-                       if ((unsigned int)ling.l_linger >= MAX_SCHEDULE_TIMEOUT/HZ)
-                               sk->sk_lingertime = MAX_SCHEDULE_TIMEOUT;
+               } else {
+                       unsigned long t_sec = ling.l_linger;
+
+                       if (t_sec >= MAX_SCHEDULE_TIMEOUT / HZ)
+                               WRITE_ONCE(sk->sk_lingertime, MAX_SCHEDULE_TIMEOUT);
                        else
-#endif
-                               sk->sk_lingertime = (unsigned int)ling.l_linger * HZ;
+                               WRITE_ONCE(sk->sk_lingertime, t_sec * HZ);
                        sock_set_flag(sk, SOCK_LINGER);
                }
                break;
@@ -1692,7 +1692,7 @@ int sk_getsockopt(struct sock *sk, int level, int optname,
        case SO_LINGER:
                lv              = sizeof(v.ling);
                v.ling.l_onoff  = sock_flag(sk, SOCK_LINGER);
-               v.ling.l_linger = sk->sk_lingertime / HZ;
+               v.ling.l_linger = READ_ONCE(sk->sk_lingertime) / HZ;
                break;
 
        case SO_BSDCOMPAT:
index 6fdba06..da34fd4 100644 (file)
@@ -502,7 +502,7 @@ META_COLLECTOR(int_sk_lingertime)
                *err = -1;
                return;
        }
-       dst->value = sk->sk_lingertime / HZ;
+       dst->value = READ_ONCE(sk->sk_lingertime) / HZ;
 }
 
 META_COLLECTOR(int_sk_err_qlen)
index c4f664f..bacdd97 100644 (file)
@@ -1840,7 +1840,7 @@ void smc_close_non_accepted(struct sock *sk)
        lock_sock(sk);
        if (!sk->sk_lingertime)
                /* wait for peer closing */
-               sk->sk_lingertime = SMC_MAX_STREAM_WAIT_TIMEOUT;
+               WRITE_ONCE(sk->sk_lingertime, SMC_MAX_STREAM_WAIT_TIMEOUT);
        __smc_release(smc);
        release_sock(sk);
        sock_put(sk); /* sock_hold above */