RE: [Patch bpf-next] unix_bpf: fix a potential deadlock in unix_dgram_bpf_recvmsg()
From: John Fastabend <john.fastabend@gmail.com>
Date: 2021-07-27 16:12:27
Cong Wang wrote:
quoted hunk ↗ jump to hunk
From: Cong Wang <redacted> As Eric noticed, __unix_dgram_recvmsg() may acquire u->iolock too, so we have to release it before calling this function. Fixes: 9825d866ce0d ("af_unix: Implement unix_dgram_bpf_recvmsg()") Reported-by: Eric Dumazet <redacted> Cc: John Fastabend <john.fastabend@gmail.com> Cc: Daniel Borkmann <daniel@iogearbox.net> Cc: Jakub Sitnicki <jakub@cloudflare.com> Cc: Lorenz Bauer <redacted> Signed-off-by: Cong Wang <redacted> --- net/unix/unix_bpf.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-)diff --git a/net/unix/unix_bpf.c b/net/unix/unix_bpf.c index db0cda29fb2f..b07cb30e87b1 100644 --- a/net/unix/unix_bpf.c +++ b/net/unix/unix_bpf.c@@ -53,8 +53,9 @@ static int unix_dgram_bpf_recvmsg(struct sock *sk, struct msghdr *msg, mutex_lock(&u->iolock); if (!skb_queue_empty(&sk->sk_receive_queue) && sk_psock_queue_empty(psock)) { - ret = __unix_dgram_recvmsg(sk, msg, len, flags); - goto out; + mutex_unlock(&u->iolock); + sk_psock_put(sk, psock); + return __unix_dgram_recvmsg(sk, msg, len, flags); }
Is there a reason to grab the mutex_lock(u->iolock) above the skb_queue_emptyaand sk_psock_queue_empty checks? Could it be move here just above the msg_bytes_ready label?
quoted hunk ↗ jump to hunk
msg_bytes_ready:@@ -68,13 +69,13 @@ static int unix_dgram_bpf_recvmsg(struct sock *sk, struct msghdr *msg, if (data) { if (!sk_psock_queue_empty(psock)) goto msg_bytes_ready; - ret = __unix_dgram_recvmsg(sk, msg, len, flags); - goto out; + mutex_unlock(&u->iolock); + sk_psock_put(sk, psock); + return __unix_dgram_recvmsg(sk, msg, len, flags); } copied = -EAGAIN; } ret = copied; -out: mutex_unlock(&u->iolock); sk_psock_put(sk, psock); return ret;-- 2.27.0