[PATCH net v2 2/2] af_unix: skip consumed OOB skb and pull consumed bytes in unix_stream_read_skb()

[Hui Peng]

When an OOB byte is consumed via recv(MSG_OOB) in unix_stream_recv_urg(),
u->oob_skb is cleared to NULL and UNIXCB(oob_skb).consumed is incremented
to 1, but oob_skb remains on sk->sk_receive_queue (with unix_skb_len(skb)
== 0) to preserve the OOB mark until normal reads advance past it.
Similarly, a partial recv() in unix_stream_read_generic() advances
UNIXCB(skb).consumed without pulling the skb header and leaves the
partially consumed skb at the head of sk->sk_receive_queue.

If the socket is subsequently read via unix_stream_read_skb() (used by
BPF sockmap), unix_stream_read_skb() only checks skb == u->oob_skb
(which is only true for an unconsumed OOB skb) and ignores
UNIXCB(skb).consumed. As a result, a consumed OOB skb (unix_skb_len(skb)
== 0) is handed to recv_actor() and re-delivers the already consumed OOB
byte, and a partially consumed skb re-delivers its already consumed
prefix.

In unix_stream_read_skb(), skip and free zero-length consumed skbs (after
calling unix_orphan_scm() so SCM_RIGHTS fd accounting remains balanced)
and pull UNIXCB(skb).consumed bytes via pskb_pull() (which safely handles
both linear and non-linear paged skbs) before invoking recv_actor().

Tested in QEMU against Linux 7.3.0-rc3 with BPF_MAP_TYPE_SOCKMAP and a
BPF_SK_SKB_STREAM_VERDICT program:
1. Consuming a 1-byte MSG_OOB packet ("Z") before inserting the socket
   into sockmap and sending "HELLO" re-delivers "ZHELLO" on the unfixed
   kernel, whereas with this patch applied recv() receives "HELLO!".
2. Consuming a 3-byte prefix ("123") of "12345678" before inserting the
   socket into sockmap re-delivers "12345678" on the unfixed kernel,
   whereas with this patch applied recv() receives "45678".

Fixes: 77462de14a43 ("af_unix: Add read_sock for stream socket types")
Fixes: 314001f0bf92 ("af_unix: Add OOB support")
Fixes: 638f32604385 ("af_unix: Disable MSG_OOB handling for sockets in sockmap/sockhash")
Cc: stable@xxxxxxxxxxxxxxx
Assisted-by: LLM
Signed-off-by: Hui Peng <benquike@xxxxxxxxx>
---
Changes in v2:
- Split out from the manage_oob() fix into patch 2/2 as requested by
  Kuniyuki Iwashima.
- Call unix_orphan_scm(sk, skb) before consume_skb(skb) when dropping a
  zero-length consumed skb so u->scm_stat.nr_fds is decremented, and use
  pskb_pull() instead of skb_pull() to safely handle non-linear paged
  skbs without hitting BUG() in __skb_pull(), as noted by Sashiko.

 net/unix/af_unix.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
index 1770af3c2684..f5d64e7b5c1c 100644
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -2885,6 +2885,7 @@ static int unix_stream_read_skb(struct sock *sk, skb_read_actor_t recv_actor)
 		return err;
 
 	mutex_lock(&u->iolock);
+again:
 	spin_lock(&queue->lock);
 
 	skb = __skb_dequeue(queue);
@@ -2894,6 +2895,13 @@ static int unix_stream_read_skb(struct sock *sk, skb_read_actor_t recv_actor)
 		return -EAGAIN;
 	}
 
+	if (!unix_skb_len(skb)) {
+		spin_unlock(&queue->lock);
+		unix_orphan_scm(sk, skb);
+		consume_skb(skb);
+		goto again;
+	}
+
 	WRITE_ONCE(u->inq_len, u->inq_len - unix_skb_len(skb));
 
 #if IS_ENABLED(CONFIG_AF_UNIX_OOB)
@@ -2913,6 +2921,14 @@ static int unix_stream_read_skb(struct sock *sk, skb_read_actor_t recv_actor)
 
 	mutex_unlock(&u->iolock);
 
+	if (UNIXCB(skb).consumed) {
+		if (!pskb_pull(skb, UNIXCB(skb).consumed)) {
+			kfree_skb(skb);
+			return -ENOMEM;
+		}
+		UNIXCB(skb).consumed = 0;
+	}
+
 	return recv_actor(sk, skb);
 }
 
-- 
2.49.0