Patch "af_unix: Clear stale u->oob_skb." has been added to the 6.1-stable tree

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

    af_unix: Clear stale u->oob_skb.

to the 6.1-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-clear-stale-u-oob_skb.patch
and it can be found in the queue-6.1 subdirectory.

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



commit 40abb36f74cf22d0ac5b4480daa6721c0e34646e
Author: Kuniyuki Iwashima <kuniyu@xxxxxxxxxx>
Date:   Fri Apr 5 15:10:57 2024 -0700

    af_unix: Clear stale u->oob_skb.
    
    [ Upstream commit b46f4eaa4f0ec38909fb0072eea3aeddb32f954e ]
    
    syzkaller started to report deadlock of unix_gc_lock after commit
    4090fa373f0e ("af_unix: Replace garbage collection algorithm."), but
    it just uncovers the bug that has been there since commit 314001f0bf92
    ("af_unix: Add OOB support").
    
    The repro basically does the following.
    
      from socket import *
      from array import array
    
      c1, c2 = socketpair(AF_UNIX, SOCK_STREAM)
      c1.sendmsg([b'a'], [(SOL_SOCKET, SCM_RIGHTS, array("i", [c2.fileno()]))], MSG_OOB)
      c2.recv(1)  # blocked as no normal data in recv queue
    
      c2.close()  # done async and unblock recv()
      c1.close()  # done async and trigger GC
    
    A socket sends its file descriptor to itself as OOB data and tries to
    receive normal data, but finally recv() fails due to async close().
    
    The problem here is wrong handling of OOB skb in manage_oob().  When
    recvmsg() is called without MSG_OOB, manage_oob() is called to check
    if the peeked skb is OOB skb.  In such a case, manage_oob() pops it
    out of the receive queue but does not clear unix_sock(sk)->oob_skb.
    This is wrong in terms of uAPI.
    
    Let's say we send "hello" with MSG_OOB, and "world" without MSG_OOB.
    The 'o' is handled as OOB data.  When recv() is called twice without
    MSG_OOB, the OOB data should be lost.
    
      >>> from socket import *
      >>> c1, c2 = socketpair(AF_UNIX, SOCK_STREAM, 0)
      >>> c1.send(b'hello', MSG_OOB)  # 'o' is OOB data
      5
      >>> c1.send(b'world')
      5
      >>> c2.recv(5)  # OOB data is not received
      b'hell'
      >>> c2.recv(5)  # OOB date is skipped
      b'world'
      >>> c2.recv(5, MSG_OOB)  # This should return an error
      b'o'
    
    In the same situation, TCP actually returns -EINVAL for the last
    recv().
    
    Also, if we do not clear unix_sk(sk)->oob_skb, unix_poll() always set
    EPOLLPRI even though the data has passed through by previous recv().
    
    To avoid these issues, we must clear unix_sk(sk)->oob_skb when dequeuing
    it from recv queue.
    
    The reason why the old GC did not trigger the deadlock is because the
    old GC relied on the receive queue to detect the loop.
    
    When it is triggered, the socket with OOB data is marked as GC candidate
    because file refcount == inflight count (1).  However, after traversing
    all inflight sockets, the socket still has a positive inflight count (1),
    thus the socket is excluded from candidates.  Then, the old GC lose the
    chance to garbage-collect the socket.
    
    With the old GC, the repro continues to create true garbage that will
    never be freed nor detected by kmemleak as it's linked to the global
    inflight list.  That's why we couldn't even notice the issue.
    
    Fixes: 314001f0bf92 ("af_unix: Add OOB support")
    Reported-by: syzbot+7f7f201cc2668a8fd169@xxxxxxxxxxxxxxxxxxxxxxxxx
    Closes: https://syzkaller.appspot.com/bug?extid=7f7f201cc2668a8fd169
    Signed-off-by: Kuniyuki Iwashima <kuniyu@xxxxxxxxxx>
    Reviewed-by: Eric Dumazet <edumazet@xxxxxxxxxx>
    Link: https://lore.kernel.org/r/20240405221057.2406-1-kuniyu@xxxxxxxxxx
    Signed-off-by: Jakub Kicinski <kuba@xxxxxxxxxx>
    Signed-off-by: Sasha Levin <sashal@xxxxxxxxxx>

diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
index e1af94393789f..373530303ad19 100644
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -2677,7 +2677,9 @@ static struct sk_buff *manage_oob(struct sk_buff *skb, struct sock *sk,
 				}
 			} else if (!(flags & MSG_PEEK)) {
 				skb_unlink(skb, &sk->sk_receive_queue);
-				consume_skb(skb);
+				WRITE_ONCE(u->oob_skb, NULL);
+				if (!WARN_ON_ONCE(skb_unref(skb)))
+					kfree_skb(skb);
 				skb = skb_peek(&sk->sk_receive_queue);
 			}
 		}




[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux