af_unix: Define locking order for U_LOCK_SECOND in unix_stream_connect().
authorKuniyuki Iwashima <kuniyu@amazon.com>
Thu, 20 Jun 2024 20:56:16 +0000 (13:56 -0700)
committerPaolo Abeni <pabeni@redhat.com>
Tue, 25 Jun 2024 09:10:18 +0000 (11:10 +0200)
While a SOCK_(STREAM|SEQPACKET) socket connect()s to another, we hold
two locks of them by unix_state_lock() and unix_state_lock_nested() in
unix_stream_connect().

Before unix_state_lock_nested(), the following is guaranteed by checking
sk->sk_state:

  1. The first socket is TCP_LISTEN
  2. The second socket is not the first one
  3. Simultaneous connect() must fail

So, the client state can be TCP_CLOSE or TCP_LISTEN or TCP_ESTABLISHED.

Let's define the expected states as unix_state_lock_cmp_fn() instead of
using unix_state_lock_nested().

Note that 2. is detected by debug_spin_lock_before() and 3. cannot be
expressed as lock_cmp_fn.

Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com>
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
include/net/af_unix.h
net/unix/af_unix.c

index b6eedf7650da594bf151c043bdcdba097173858c..fd813ad73ab863e0d9499485dd775d99a35d76bd 100644 (file)
@@ -98,7 +98,6 @@ struct unix_sock {
 #define unix_state_unlock(s)   spin_unlock(&unix_sk(s)->lock)
 enum unix_socket_lock_class {
        U_LOCK_NORMAL,
-       U_LOCK_SECOND,  /* for double locking, see unix_state_double_lock(). */
        U_LOCK_DIAG, /* used while dumping icons, see sk_diag_dump_icons(). */
        U_LOCK_GC_LISTENER, /* used for listening socket while determining gc
                             * candidates to close a small race window.
index 88f2c5d039c4a8d7060530386723a27363f75977..a092d6999ae02b50a5f589452f34e98b3cf7ffba 100644 (file)
@@ -143,6 +143,41 @@ static int unix_state_lock_cmp_fn(const struct lockdep_map *_a,
        a = container_of(_a, struct unix_sock, lock.dep_map);
        b = container_of(_b, struct unix_sock, lock.dep_map);
 
+       if (a->sk.sk_state == TCP_LISTEN) {
+               /* unix_stream_connect(): Before the 2nd unix_state_lock(),
+                *
+                *   1. a is TCP_LISTEN.
+                *   2. b is not a.
+                *   3. concurrent connect(b -> a) must fail.
+                *
+                * Except for 2. & 3., the b's state can be any possible
+                * value due to concurrent connect() or listen().
+                *
+                * 2. is detected in debug_spin_lock_before(), and 3. cannot
+                * be expressed as lock_cmp_fn.
+                */
+               switch (b->sk.sk_state) {
+               case TCP_CLOSE:
+               case TCP_ESTABLISHED:
+               case TCP_LISTEN:
+                       return -1;
+               default:
+                       /* Invalid case. */
+                       return 0;
+               }
+       }
+
+       /* Should never happen.  Just to be symmetric. */
+       if (b->sk.sk_state == TCP_LISTEN) {
+               switch (b->sk.sk_state) {
+               case TCP_CLOSE:
+               case TCP_ESTABLISHED:
+                       return 1;
+               default:
+                       return 0;
+               }
+       }
+
        /* unix_state_double_lock(): ascending address order. */
        return cmp_ptr(a, b);
 }
@@ -1585,7 +1620,7 @@ restart:
                goto out_unlock;
        }
 
-       unix_state_lock_nested(sk, U_LOCK_SECOND);
+       unix_state_lock(sk);
 
        if (unlikely(sk->sk_state != TCP_CLOSE)) {
                err = sk->sk_state == TCP_ESTABLISHED ? -EISCONN : -EINVAL;