3 messages, 2 authors, 2021-08-13 · open the first message on its own page
From: Rao Shoaib <hidden>
Date: 2021-08-13 17:58:35
Resending with updated version number. Addresses spinlock issue found by syzkaller and comments from edumazet@google.com. Rao Shoaib (1): af_unix: fix holding spinlock in oob handling net/unix/af_unix.c | 47 ++++++++++++++++++++++++++++++++-------------- 1 file changed, 33 insertions(+), 14 deletions(-) -- 2.27.0
From: Rao Shoaib <hidden>
Date: 2021-08-13 17:58:37
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
From: Jakub Kicinski <kuba@kernel.org>
Date: 2021-08-13 18:01:57
On Fri, 13 Aug 2021 10:58:16 -0700 Rao Shoaib wrote:
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.
I applied v1 an hour or so ago, please resend addressing only the issues pointed out by Eric. Try to separate the issues each into its own patch. That's easier to review.