Re: [PATCH] af_unix: fix holding spinlock in oob handling

On Thu, Aug 12, 2021 at 12:07 AM Rao Shoaib [off-list ref] wrote:

From: Rao Shoaib <redacted>

syzkaller found that OOB code was holding spinlock
while calling a function in which it could sleep.

Reported-by: syzbot+8760ca6c1ee783ac4abd@syzkaller.appspotmail.com

Fixes: 314001f0bf92 ("af_unix: Add OOB support")

Signed-off-by: Rao Shoaib <redacted>
---

Please do not add these empty lines.

Fixes: ...
Reported-by: ...
Signed-off-by: ...

Also you might take a look at queue_oob()

1)  Setting skb->len tp 1 should not be needed, skb_put() already does that
2) After unix_state_lock(other); we probably need to check status of
the other socket.
3) Some skb_free() calls should have been consume_skb()

diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
index ec02e70a549b42f6c102253508c48426a13f7bc4..0c27e2976f9d234ca3bb131731375bc51a056846

100644

--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c

@@ -1908,7 +1908,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) {

@@ -1917,11 +1916,17 @@ 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;