From 9d99b7e92e0b8c7f16a36549e96413195d3ebfb7 Mon Sep 17 00:00:00 2001 From: Kuniyuki Iwashima Date: Mon, 24 Jun 2024 18:36:38 -0700 Subject: [PATCH] af_unix: Don't stop recv(MSG_DONTWAIT) if consumed OOB skb is at the head. BugLink: https://bugs.launchpad.net/bugs/2076435 [ 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 "", line 1, in 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 Signed-off-by: Paolo Abeni Signed-off-by: Sasha Levin Signed-off-by: Portia Stephens Signed-off-by: Roxana Nicolescu --- net/unix/af_unix.c | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c index f4e99731cacb..a8364cf18f6f 100644 --- a/net/unix/af_unix.c +++ b/net/unix/af_unix.c @@ -2600,12 +2600,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;