Patch "af_unix: Don't stop recv(MSG_DONTWAIT) if consumed OOB skb is at the head." has been added to the 6.9-stable tree

[Sasha Levin <sashal@xxxxxxxxxx>]

This is a note to let you know that I've just added the patch titled

    af_unix: Don't stop recv(MSG_DONTWAIT) if consumed OOB skb is at the head.

to the 6.9-stable tree which can be found at:
    http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary

The filename of the patch is:
     af_unix-don-t-stop-recv-msg_dontwait-if-consumed-oob.patch
and it can be found in the queue-6.9 subdirectory.

If you, or anyone else, feels it should not be added to the stable tree,
please let <stable@xxxxxxxxxxxxxxx> know about it.



commit 4dd8fcab9527dcb2e90530feeff507257c53ce23
Author: Kuniyuki Iwashima <kuniyu@xxxxxxxxxx>
Date:   Mon Jun 24 18:36:38 2024 -0700

    af_unix: Don't stop recv(MSG_DONTWAIT) if consumed OOB skb is at the head.
    
    [ Upstream commit 93c99f21db360957d49853e5666b5c147f593bda ]
    
    Let's say a socket send()s "hello" with MSG_OOB and "world" without flags,
    
      >>> from socket import *
      >>> c1, c2 = socketpair(AF_UNIX)
      >>> c1.send(b'hello', MSG_OOB)
      5
      >>> c1.send(b'world')
      5
    
    and its peer recv()s "hell" and "o".
    
      >>> c2.recv(10)
      b'hell'
      >>> c2.recv(1, MSG_OOB)
      b'o'
    
    Now the consumed OOB skb stays at the head of recvq to return a correct
    value for ioctl(SIOCATMARK), which is broken now and fixed by a later
    patch.
    
    Then, if peer issues recv() with MSG_DONTWAIT, manage_oob() returns NULL,
    so recv() ends up with -EAGAIN.
    
      >>> c2.setblocking(False)  # This causes -EAGAIN even with available data
      >>> c2.recv(5)
      Traceback (most recent call last):
        File "<stdin>", line 1, in <module>
      BlockingIOError: [Errno 11] Resource temporarily unavailable
    
    However, next recv() will return the following available data, "world".
    
      >>> c2.recv(5)
      b'world'
    
    When the consumed OOB skb is at the head of the queue, we need to fetch
    the next skb to fix the weird behaviour.
    
    Note that the issue does not happen without MSG_DONTWAIT because we can
    retry after manage_oob().
    
    This patch also adds a test case that covers the issue.
    
    Without fix:
    
      #  RUN           msg_oob.no_peek.ex_oob_break ...
      # msg_oob.c:134:ex_oob_break:AF_UNIX :Resource temporarily unavailable
      # msg_oob.c:135:ex_oob_break:Expected:ld
      # msg_oob.c:137:ex_oob_break:Expected ret[0] (-1) == expected_len (2)
      # ex_oob_break: Test terminated by assertion
      #          FAIL  msg_oob.no_peek.ex_oob_break
      not ok 8 msg_oob.no_peek.ex_oob_break
    
    With fix:
    
      #  RUN           msg_oob.no_peek.ex_oob_break ...
      #            OK  msg_oob.no_peek.ex_oob_break
      ok 8 msg_oob.no_peek.ex_oob_break
    
    Fixes: 314001f0bf92 ("af_unix: Add OOB support")
    Signed-off-by: Kuniyuki Iwashima <kuniyu@xxxxxxxxxx>
    Signed-off-by: Paolo Abeni <pabeni@xxxxxxxxxx>
    Signed-off-by: Sasha Levin <sashal@xxxxxxxxxx>

diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
index d687670e84990..645ac77e4dda3 100644
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -2661,12 +2661,23 @@ static struct sk_buff *manage_oob(struct sk_buff *skb, struct sock *sk,
 	struct unix_sock *u = unix_sk(sk);
 
 	if (!unix_skb_len(skb)) {
-		if (!(flags & MSG_PEEK)) {
-			skb_unlink(skb, &sk->sk_receive_queue);
-			consume_skb(skb);
+		struct sk_buff *unlinked_skb = NULL;
+
+		spin_lock(&sk->sk_receive_queue.lock);
+
+		if (copied) {
+			skb = NULL;
+		} else if (flags & MSG_PEEK) {
+			skb = skb_peek_next(skb, &sk->sk_receive_queue);
+		} else {
+			unlinked_skb = skb;
+			skb = skb_peek_next(skb, &sk->sk_receive_queue);
+			__skb_unlink(unlinked_skb, &sk->sk_receive_queue);
 		}
 
-		skb = NULL;
+		spin_unlock(&sk->sk_receive_queue.lock);
+
+		consume_skb(unlinked_skb);
 	} else {
 		struct sk_buff *unlinked_skb = NULL;