diff options
| author | Kent Overstreet <kent.overstreet@linux.dev> | 2023-12-22 23:47:14 -0500 |
|---|---|---|
| committer | Kent Overstreet <kent.overstreet@linux.dev> | 2023-12-22 23:49:37 -0500 |
| commit | 3d02c2fdf870288589fe08d29e1255584f0d796e (patch) | |
| tree | 45f52f6da4efb4c6a4c7a9cae5597d4bd83a0929 | |
| parent | c86373faa01918197f4996eb155e188a0ab5b58b (diff) | |
af_unix: convert to lock_cmp_fnlockdep_cmp_fn
It turns out there's a deadlock in this code that lockdep couldn't
detect because of incorrect use of nested locking.
Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
| -rw-r--r-- | include/net/af_unix.h | 3 | ||||
| -rw-r--r-- | net/unix/af_unix.c | 17 | ||||
| -rw-r--r-- | net/unix/diag.c | 4 |
3 files changed, 9 insertions, 15 deletions
diff --git a/include/net/af_unix.h b/include/net/af_unix.h index 49c4640027d8..4eff0a089640 100644 --- a/include/net/af_unix.h +++ b/include/net/af_unix.h @@ -48,9 +48,6 @@ struct scm_stat { #define unix_state_lock(s) spin_lock(&unix_sk(s)->lock) #define unix_state_unlock(s) spin_unlock(&unix_sk(s)->lock) -#define unix_state_lock_nested(s) \ - spin_lock_nested(&unix_sk(s)->lock, \ - SINGLE_DEPTH_NESTING) /* The AF_UNIX socket */ struct unix_sock { diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c index 1587cfe0f881..1a0d273799c1 100644 --- a/net/unix/af_unix.c +++ b/net/unix/af_unix.c @@ -997,6 +997,7 @@ static struct sock *unix_create1(struct net *net, struct socket *sock, int kern, u->path.dentry = NULL; u->path.mnt = NULL; spin_lock_init(&u->lock); + lock_set_cmp_fn_ptr_order(&u->lock); atomic_long_set(&u->inflight, 0); INIT_LIST_HEAD(&u->link); mutex_init(&u->iolock); /* single task reading lock */ @@ -1340,17 +1341,11 @@ static int unix_bind(struct socket *sock, struct sockaddr *uaddr, int addr_len) static void unix_state_double_lock(struct sock *sk1, struct sock *sk2) { - if (unlikely(sk1 == sk2) || !sk2) { - unix_state_lock(sk1); - return; - } - if (sk1 < sk2) { + if (sk1 > sk2) + swap(sk1, sk2); + if (sk1 && sk1 != sk2) unix_state_lock(sk1); - unix_state_lock_nested(sk2); - } else { - unix_state_lock(sk2); - unix_state_lock_nested(sk1); - } + unix_state_lock(sk2); } static void unix_state_double_unlock(struct sock *sk1, struct sock *sk2) @@ -1591,7 +1586,7 @@ restart: goto out_unlock; } - unix_state_lock_nested(sk); + unix_state_lock(sk); if (sk->sk_state != st) { unix_state_unlock(sk); diff --git a/net/unix/diag.c b/net/unix/diag.c index bec09a3a1d44..c2b0fc72f1f7 100644 --- a/net/unix/diag.c +++ b/net/unix/diag.c @@ -83,8 +83,10 @@ static int sk_diag_dump_icons(struct sock *sk, struct sk_buff *nlskb) * The state lock is outer for the same sk's * queue lock. With the other's queue locked it's * OK to lock the state. + * ??? + * What is the previous lock, what is our lock ordering? */ - unix_state_lock_nested(req); + unix_state_lock(req); peer = unix_sk(req)->peer; buf[i++] = (peer ? sock_i_ino(peer) : 0); unix_state_unlock(req); |