[PATCH v2 1/1] af_unix: fix holding spinlock in oob handling
From: Rao Shoaib <hidden>
Date: 2021-08-13 17:58:37
Subsystem:
networking [general], networking [unix sockets], the rest · Maintainers:
"David S. Miller", Eric Dumazet, Jakub Kicinski, Paolo Abeni, Kuniyuki Iwashima, Linus Torvalds
From: Rao Shoaib <redacted>
syzkaller found that OOB code was holding spinlock
while calling a function in which it could sleep.
Also addressed comments from edumazet@google.com.
Reported-by: syzbot+8760ca6c1ee783ac4abd@syzkaller.appspotmail.com
Fixes: 314001f0bf92 ("af_unix: Add OOB support")
Signed-off-by: Rao Shoaib <redacted>
---
net/unix/af_unix.c | 47 ++++++++++++++++++++++++++++++++--------------
1 file changed, 33 insertions(+), 14 deletions(-)
diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
index 00d8b08cdbe1..0f59fed993d8 100644
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c@@ -1891,7 +1891,6 @@ static int queue_oob(struct socket *sock, struct msghdr *msg, struct sock *other return err; skb_put(skb, 1); - skb->len = 1; err = skb_copy_datagram_from_iter(skb, 0, &msg->msg_iter, 1); if (err) {
@@ -1900,11 +1899,19 @@ static int queue_oob(struct socket *sock, struct msghdr *msg, struct sock *other } unix_state_lock(other); + + if (sock_flag(other, SOCK_DEAD) || + (other->sk_shutdown & RCV_SHUTDOWN)) { + unix_state_unlock(other); + kfree_skb(skb); + return -EPIPE; + } + maybe_add_creds(skb, sock, other); skb_get(skb); if (ousk->oob_skb) - kfree_skb(ousk->oob_skb); + consume_skb(ousk->oob_skb); ousk->oob_skb = skb;
@@ -2362,19 +2369,37 @@ static int unix_stream_recv_urg(struct unix_stream_read_state *state) struct sock *sk = sock->sk; struct unix_sock *u = unix_sk(sk); int chunk = 1; + struct sk_buff *oob_skb; + + mutex_lock(&u->iolock); + unix_state_lock(sk); - if (sock_flag(sk, SOCK_URGINLINE) || !u->oob_skb) + if (sock_flag(sk, SOCK_URGINLINE) || !u->oob_skb) { + unix_state_unlock(sk); + mutex_unlock(&u->iolock); return -EINVAL; + } - chunk = state->recv_actor(u->oob_skb, 0, chunk, state); - if (chunk < 0) - return -EFAULT; + oob_skb = u->oob_skb; if (!(state->flags & MSG_PEEK)) { - UNIXCB(u->oob_skb).consumed += 1; - kfree_skb(u->oob_skb); u->oob_skb = NULL; } + + unix_state_unlock(sk); + + chunk = state->recv_actor(oob_skb, 0, chunk, state); + + if (!(state->flags & MSG_PEEK)) { + UNIXCB(oob_skb).consumed += 1; + kfree_skb(oob_skb); + } + + mutex_unlock(&u->iolock); + + if (chunk < 0) + return -EFAULT; + state->msg->msg_flags |= MSG_OOB; return 1; }
@@ -2434,13 +2459,7 @@ static int unix_stream_read_generic(struct unix_stream_read_state *state, if (unlikely(flags & MSG_OOB)) { err = -EOPNOTSUPP; #if IS_ENABLED(CONFIG_AF_UNIX_OOB) - mutex_lock(&u->iolock); - unix_state_lock(sk); - err = unix_stream_recv_urg(state); - - unix_state_unlock(sk); - mutex_unlock(&u->iolock); #endif goto out; }
--
2.27.0