[PATCH] af_unix: fix u->inq_len accounting in manage_oob() and unix_stream_read_skb()

[PATCH] af_unix: fix u->inq_len accounting in manage_oob() and unix_stream_read_skb()

[Hui Peng]

When an old OOB skb is replaced and discarded in manage_oob(), or when a
fully consumed OOB skb is skipped in unix_stream_read_skb(), the
discarded byte count is not subtracted from u->inq_len (or is subtracted
inconsistently), causing SIOCINQ / FIONREAD to report stale positive
byte counts on an empty AF_UNIX stream socket. Properly account
u->inq_len when dropping or skipping consumed OOB skbs.

Fixes: 314001f0bf92 ("af_unix: Add OOB support")
Assisted-by: LLM
Signed-off-by: Hui Peng <benquike@gmail.com>
---
diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
index 42cffeafc8c1..1770af3c2684 100644
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -2849,6 +2849,7 @@ static struct sk_buff *manage_oob(struct sk_buff *skb, struct sk_buff **last,
 		WRITE_ONCE(u->oob_skb, NULL);
 
 		if (!sock_flag(sk, SOCK_URGINLINE)) {
+			WRITE_ONCE(u->inq_len, u->inq_len - 1);
 			__skb_unlink(skb, &sk->sk_receive_queue);
 			unread_skb = skb;
 			skb = skb_peek(&sk->sk_receive_queue);
@@ -2884,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);
@@ -2893,6 +2895,12 @@ 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);
+		consume_skb(skb);
+		goto again;
+	}
+
 	WRITE_ONCE(u->inq_len, u->inq_len - unix_skb_len(skb));
 
 #if IS_ENABLED(CONFIG_AF_UNIX_OOB)
@@ -2908,6 +2916,11 @@ static int unix_stream_read_skb(struct sock *sk, skb_read_actor_t recv_actor)
 
 	spin_unlock(&queue->lock);
 
+	if (UNIXCB(skb).consumed) {
+		skb_pull(skb, UNIXCB(skb).consumed);
+		UNIXCB(skb).consumed = 0;
+	}
+
 	unix_orphan_scm(sk, skb);
 
 	mutex_unlock(&u->iolock);

Re: [PATCH] af_unix: fix u->inq_len accounting in manage_oob() and unix_stream_read_skb()

[Kuniyuki Iwashima]

On Sat, Sep 19, 2026 at 3:17 PM Hui Peng <benquike@gmail.com> wrote:
>
> When an old OOB skb is replaced and discarded in manage_oob(), or when a
> fully consumed OOB skb is skipped in unix_stream_read_skb(), the
> discarded byte count is not subtracted from u->inq_len (or is subtracted
> inconsistently), causing SIOCINQ / FIONREAD to report stale positive
> byte counts on an empty AF_UNIX stream socket. Properly account
> u->inq_len when dropping or skipping consumed OOB skbs.
>
> Fixes: 314001f0bf92 ("af_unix: Add OOB support")
> Assisted-by: LLM

Please tell your LLM not to mix different fixes into one.

btw I'm tempted to remove OOB support entirely.

---
pw-bot: cr

Re: [PATCH] af_unix: fix u->inq_len accounting in manage_oob() and unix_stream_read_skb()

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 4 potential
issue(s) to consider.

Critical: 1 · High: 0 · Medium: 3 · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Critical] Kernel BUG() (oops/panic) in the sockmap read path: the
  newly added `skb_pull(skb, UNIXCB(skb).consumed)` in…
- [Medium] Commit message does not match the code, and two of the three
  hunks are unrelated to inq_len accounting (scope creep).
- [Medium] Incorrect Fixes tag / wrong bug attribution.
- [Medium] Unbalanced SCM_RIGHTS accounting on the new early-drop path in
  `unix_stream_read_skb()`: the added block frees the dequeued skb with…

--

af_unix: fix u->inq_len accounting in manage_oob() and unix_stream_read_skb()

This adds a u->inq_len decrement when manage_oob() discards a replaced OOB
skb, makes unix_stream_read_skb() skip and free fully consumed skbs, and
pulls the already consumed prefix out of the skb before handing it to the
recv_actor.  No Link: tags are present in the commit header.

A few questions on the changelog and on the two new blocks in
unix_stream_read_skb().

> When an old OOB skb is replaced and discarded in manage_oob(), or when a
> fully consumed OOB skb is skipped in unix_stream_read_skb(), the
> discarded byte count is not subtracted from u->inq_len (or is subtracted
> inconsistently), causing SIOCINQ / FIONREAD to report stale positive
> byte counts on an empty AF_UNIX stream socket.

[Severity: Medium]
Is the unix_stream_read_skb() half of this description accurate?

unix_skb_len() is:

    static unsigned int unix_skb_len(const struct sk_buff *skb)
    {
    	return skb->len - UNIXCB(skb).consumed;
    }

so a fully consumed skb contributes 0 bytes, and the pre-existing
subtraction already in that function:

    WRITE_ONCE(u->inq_len, u->inq_len - unix_skb_len(skb));

subtracts exactly 0 for it.  The bytes were already removed either by the
partial read in unix_stream_read_generic():

    WRITE_ONCE(u->inq_len, u->inq_len - chunk);

or by unix_stream_recv_urg():

    WRITE_ONCE(u->inq_len, u->inq_len - 1);

Can a fully consumed skb still hold un-accounted bytes in this path?

The new !unix_skb_len(skb) block contains no inq_len update at all, and the
new skb_pull() block is not mentioned in the changelog.  Both change what
the ->read_skb() (BPF sockmap) consumer sees: one stops delivery of a
zero-payload skb, the other stops re-delivering already consumed bytes.
Could those two changes be split out with their own justification, so that
this patch only carries the manage_oob() accounting fix?

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

[Severity: Medium]
Is this the right Fixes target?  u->inq_len does not exist at that commit;
blame on include/net/af_unix.h attributes

    int			inq_len;

to f4e1fb04c1238 ("af_unix: Use cached value for SOCK_STREAM in
unix_inq_len().").  Before that, unix_inq_len() walked the receive queue:

    skb_queue_walk(&sk->sk_receive_queue, skb)
    	amount += unix_skb_len(skb);

so a discarded OOB skb could not leave a stale count behind.  f4e1fb04c1238
added the inq_len increment in queue_oob() and the decrement in
unix_stream_recv_urg(), but not the manage_oob() discard decrement this
patch adds.  Should the tag point at f4e1fb04c1238 instead, and should the
patch carry Cc: stable@vger.kernel.org given it claims a user-visible
SIOCINQ/FIONREAD fix?

> diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
> index 42cffeafc8c1..1770af3c2684 100644
> --- a/net/unix/af_unix.c
> +++ b/net/unix/af_unix.c
> @@ -2849,6 +2849,7 @@ static struct sk_buff *manage_oob(struct sk_buff *skb, struct sk_buff **last,
>  		WRITE_ONCE(u->oob_skb, NULL);
>  
>  		if (!sock_flag(sk, SOCK_URGINLINE)) {
> +			WRITE_ONCE(u->inq_len, u->inq_len - 1);
>  			__skb_unlink(skb, &sk->sk_receive_queue);
>  			unread_skb = skb;
>  			skb = skb_peek(&sk->sk_receive_queue);
> @@ -2884,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);
> @@ -2893,6 +2895,12 @@ 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);
> +		consume_skb(skb);
> +		goto again;
> +	}
> +

[Severity: Medium]
Does this new discard path leave u->scm_stat.nr_fds inflated?

The normal exit of unix_stream_read_skb() goes through:

    static void unix_orphan_scm(struct sock *sk, struct sk_buff *skb)
    {
    	scm_stat_del(sk, skb);
    	unix_destruct_scm(skb);
    	skb->destructor = sock_wfree;
    }

and scm_stat_del() is the only place that decrements nr_fds:

    if (unlikely(fp && fp->count)) {
    	atomic_sub(fp->count, &u->scm_stat.nr_fds);
    	unix_del_edges(fp);
    }

The bare consume_skb() here goes through the destructor chain
unix_wfree() -> unix_destruct_scm() -> unix_detach_fds(), which releases the
files but never touches scm_stat.nr_fds.

A reachable input looks like: sendmsg(MSG_OOB) with SCM_RIGHTS, where
queue_oob() attaches the fds and calls scm_stat_add(other, skb); then a
recvmsg(MSG_OOB) without MSG_PEEK in unix_stream_recv_urg() does

    if (!(state->flags & MSG_PEEK))
    	UNIXCB(oob_skb).consumed += 1;

leaving a queued zero-payload skb that still carries UNIXCB(skb).fp.  When
that skb reaches the head of the queue on a sockmap'd socket, the new branch
frees it without scm_stat_del(), so unix_show_fdinfo() keeps reporting an
inflated scm_fds for the life of the socket.  Would calling
unix_orphan_scm(sk, skb) (or at least scm_stat_del()) before freeing keep
the accounting balanced?

>  	WRITE_ONCE(u->inq_len, u->inq_len - unix_skb_len(skb));
>  
>  #if IS_ENABLED(CONFIG_AF_UNIX_OOB)
> @@ -2908,6 +2916,11 @@ static int unix_stream_read_skb(struct sock *sk, skb_read_actor_t recv_actor)
>  
>  	spin_unlock(&queue->lock);
>  
> +	if (UNIXCB(skb).consumed) {
> +		skb_pull(skb, UNIXCB(skb).consumed);
> +		UNIXCB(skb).consumed = 0;
> +	}
> +

[Severity: Critical]
Can this skb_pull() hit the BUG() in __skb_pull() on a paged skb?

UNIXCB(skb).consumed is a logical stream offset, not bounded by
skb_headlen(skb).  unix_stream_sendmsg() routinely builds non-linear skbs:

    data_len = max_t(int, 0, size - SKB_MAX_HEAD(0));
    ...
    skb = sock_alloc_send_pskb(sk, size - data_len, data_len, ...);

and on the MSG_SPLICE_PAGES branch the head length is 0.

unix_stream_read_generic() then advances consumed and deliberately leaves
the skb queued while any payload remains:

    UNIXCB(skb).consumed += chunk;
    ...
    if (unix_skb_len(skb)) {
    	spin_unlock(&sk->sk_receive_queue.lock);
    	break;
    }

skb_pull() only rejects len > skb->len; __skb_pull() then does:

    skb->len -= len;
    if (unlikely(skb->len < skb->data_len)) {
    	...
    	BUG();
    }

So a peer writing more than SKB_MAX_HEAD(0) bytes (or splicing pages, where
a single consumed byte is enough), a reader consuming a prefix past the
linear head with a normal recvmsg(), and then further data arriving on a
socket with a sockmap verdict program gives
sk_psock_verdict_data_ready() -> ops->read_skb() -> unix_stream_read_skb() ->
skb_pull() -> BUG().

The BUG() fires after spin_unlock(&queue->lock) but before
mutex_unlock(&u->iolock), so the task dies holding u->iolock and further
readers and MSG_OOB users of that socket block forever; the skb is already
dequeued and u->inq_len already decremented.

If already consumed bytes need to be hidden from the recv_actor, would a
paged-safe form such as pskb_pull()/__pskb_pull_tail() with failure handling,
or passing the offset to the actor, be more appropriate here?

>  	unix_orphan_scm(sk, skb);
>  
>  	mutex_unlock(&u->iolock);

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260919221735.3708237-1-benquike%40gmail.com

[PATCH net v2 1/2] af_unix: decrement u->inq_len when skipping unread OOB skb in manage_oob()

[Hui Peng]

When a normal in-band read on an AF_UNIX stream socket (without MSG_PEEK
and without SO_OOBINLINE) encounters an unread u->oob_skb in manage_oob(),
manage_oob() clears u->oob_skb, unlinks the 1-byte skb from
sk->sk_receive_queue, and drops it with SKB_DROP_REASON_UNIX_SKIP_OOB
without decrementing u->inq_len. Because queue_oob() incremented
u->inq_len by 1 when queuing the OOB skb, u->inq_len remains permanently
inflated by 1 byte for each skipped unread OOB skb, causing SIOCINQ /
FIONREAD to report a stale positive byte count on an empty socket.

Decrement u->inq_len by 1 when unlinking the unread OOB skb in
manage_oob().

Tested in QEMU against Linux 7.3.0-rc3 by sending three 1-byte MSG_OOB
packets interleaved with normal stream data on an AF_UNIX SOCK_STREAM
socketpair and draining all in-band data via recv(): on the unfixed
kernel, ioctl(SIOCINQ) reports 3 on the empty socket; with this patch
applied, ioctl(SIOCINQ) reports 0.

Fixes: f4e1fb04c123 ("af_unix: Use cached value for SOCK_STREAM in unix_inq_len().")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Hui Peng <benquike@gmail.com>
---
Changes in v2:
- Split the manage_oob() and unix_stream_read_skb() fixes into a 2-patch
  series as requested by Kuniyuki Iwashima.
- Update Fixes: tag to f4e1fb04c123 ("af_unix: Use cached value for
  SOCK_STREAM in unix_inq_len().") and clarify the commit message as
  noted by Sashiko.

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

diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
index 42cffeafc8c1..1770af3c2684 100644
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -2849,6 +2849,7 @@ static struct sk_buff *manage_oob(struct sk_buff *skb, struct sk_buff **last,
 		WRITE_ONCE(u->oob_skb, NULL);
 
 		if (!sock_flag(sk, SOCK_URGINLINE)) {
+			WRITE_ONCE(u->inq_len, u->inq_len - 1);
 			__skb_unlink(skb, &sk->sk_receive_queue);
 			unread_skb = skb;
 			skb = skb_peek(&sk->sk_receive_queue);
-- 
2.49.0

[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@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Hui Peng <benquike@gmail.com>
---
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

[syzbot ci] Re: af_unix: fix u->inq_len accounting in manage_oob() and unix_stream_read_skb()

[syzbot ci]

syzbot ci has tested the following series

[v1] af_unix: fix u->inq_len accounting in manage_oob() and unix_stream_read_skb()
https://lore.kernel.org/all/20260919221735.3708237-1-benquike@gmail.com
* [PATCH] af_unix: fix u->inq_len accounting in manage_oob() and unix_stream_read_skb()

and found the following issue:
kernel BUG in unix_stream_read_skb

Full report is available here:
https://ci.syzbot.org/series/0ffad94e-8c51-41d9-b85f-9793ff3deaf5

***

kernel BUG in unix_stream_read_skb

tree:      net-next
URL:       https://kernel.googlesource.com/pub/scm/linux/kernel/git/netdev/net-next.git
base:      b5a051f6b840d48f159166ef073d3021989bfb50
arch:      amd64
compiler:  Debian clang version 22.1.8 (++20260613092233+e80beda6e255-1~exp1~20260613092250.77), Debian LLD 22.1.8
config:    https://ci.syzbot.org/builds/127895ae-9f2e-46db-a972-bc6f975448e6/config

skb linear:   00000100: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
skb linear:   00000110: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
skb linear:   00000120: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
------------[ cut here ]------------
kernel BUG at ./include/linux/skbuff.h:2847!
Oops: invalid opcode: 0000 [#1] SMP KASAN PTI
CPU: 1 UID: 0 PID: 16941 Comm: syz.0.3527 Not tainted syzkaller #0 PREEMPT(full) 
Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.2-debian-1.16.2-1 04/01/2014
RIP: 0010:skb_pull+0x1c4/0x1d0
Code: b2 8c f8 8b 44 24 04 89 43 70 48 c7 c7 c0 ba 22 8d 89 ee e8 9e 96 7c f7 48 c7 c7 00 bb 22 8d 48 89 de 31 d2 e8 7d 00 ff ff 90 <0f> 0b 66 2e 0f 1f 84 00 00 00 00 00 90 90 90 90 90 90 90 90 90 90
RSP: 0018:ffffc9000629f758 EFLAGS: 00010282
RAX: ffffffff89aa3aaa RBX: ffff888107756780 RCX: 0000000000080000
RDX: ffffc90021fe7000 RSI: 000000000000b6ec RDI: 000000000000b6ed
RBP: 0000000000001000 R08: ffff88823c624713 R09: 1ffff110478c48e2
R10: dffffc0000000000 R11: ffffed10478c48e3 R12: 0000000000008000
R13: dffffc0000000000 R14: 1ffff11020eeacfe R15: 0000000000007ec0
FS:  00007f4d8a7ef6c0(0000) GS:ffff8882a8cce000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00002000000c4000 CR3: 000000002253a000 CR4: 00000000000006f0
Call Trace:
 <TASK>
 unix_stream_read_skb+0x34a/0x500
 sk_psock_verdict_data_ready+0x25f/0x690
 unix_stream_sendmsg+0x84e/0xde0
 sock_sendmsg_nosec+0x13a/0x180
 ____sys_sendmsg+0x54e/0x850
 ___sys_sendmsg+0x2a5/0x360
 __x64_sys_sendmsg+0x1b1/0x290
 do_syscall_64+0x166/0x520
 entry_SYSCALL_64_after_hwframe+0x77/0x7f
RIP: 0033:0x7f4d8999e159
Code: ff c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 44 00 00 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 c7 c1 e8 ff ff ff f7 d8 64 89 01 48
RSP: 002b:00007f4d8a7ef028 EFLAGS: 00000246 ORIG_RAX: 000000000000002e
RAX: ffffffffffffffda RBX: 00007f4d89c26180 RCX: 00007f4d8999e159
RDX: 0000000000000000 RSI: 0000200000000500 RDI: 0000000000000005
RBP: 00007f4d89a3506b R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000000
R13: 00007f4d89c26218 R14: 00007f4d89c26180 R15: 00007ffd1de3bf98
 </TASK>
Modules linked in:
---[ end trace 0000000000000000 ]---
RIP: 0010:skb_pull+0x1c4/0x1d0
Code: b2 8c f8 8b 44 24 04 89 43 70 48 c7 c7 c0 ba 22 8d 89 ee e8 9e 96 7c f7 48 c7 c7 00 bb 22 8d 48 89 de 31 d2 e8 7d 00 ff ff 90 <0f> 0b 66 2e 0f 1f 84 00 00 00 00 00 90 90 90 90 90 90 90 90 90 90
RSP: 0018:ffffc9000629f758 EFLAGS: 00010282
RAX: ffffffff89aa3aaa RBX: ffff888107756780 RCX: 0000000000080000
RDX: ffffc90021fe7000 RSI: 000000000000b6ec RDI: 000000000000b6ed
RBP: 0000000000001000 R08: ffff88823c624713 R09: 1ffff110478c48e2
R10: dffffc0000000000 R11: ffffed10478c48e3 R12: 0000000000008000
R13: dffffc0000000000 R14: 1ffff11020eeacfe R15: 0000000000007ec0
FS:  00007f4d8a7ef6c0(0000) GS:ffff88818d6ce000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007fda6ffeb840 CR3: 000000002253a000 CR4: 00000000000006f0


***

If these findings have caused you to resend the series or submit a
separate fix, please add the following tag to your commit message:
  Tested-by: syzbot@syzkaller.appspotmail.com

---
This report is generated by a bot. It may contain errors.
syzbot ci engineers can be reached at syzkaller@googlegroups.com.

To test a fix for this bug, please reply with `#syz test`
(on a separate line) and attach the patch to the email.

Notes:
- The patch will be applied on top of the tested series (as an
  incremental fix).
- To test a new version of the whole series, please send it directly
  to syzbot@lists.linux.dev.
- Arguments like custom git repos and branches are not supported.

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

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider.

Critical: 0 · High: 1 · Medium: 0 (1) · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [High] net/unix/af_unix.c, unix_stream_read_skb(): the newly added pull
  block frees the whole SKB and returns -ENOMEM when pskb_pull(skb,…

Pre-existing issues:
- [Medium] Unbalanced u->scm_stat.nr_fds accounting on AF_UNIX skb drop
  paths.

--

af_unix: skip consumed OOB skb and pull consumed bytes in unix_stream_read_skb()

This changes unix_stream_read_skb() to drop zero-length skbs left behind
after an OOB byte was consumed, and to pull UNIXCB(skb).consumed bytes off
a partially consumed skb before handing it to recv_actor().  Without this,
sockmap readers re-deliver bytes that a previous recv() already consumed.

> diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
> index 0903bd587696..a1c06b9995c7 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;
> +	}
> +

[Severity: Medium]
This isn't a bug introduced by this patch, but the new block here uses the
correct pairing (unix_orphan_scm() before consume_skb()) while several
neighbouring free paths for the same class of skb do not, so it highlights
an existing imbalance in u->scm_stat.nr_fds.

scm_stat_del(), reached only from unix_orphan_scm(), is the only place that
does the matching decrement:

net/unix/af_unix.c:scm_stat_del() {
	...
	if (unlikely(fp && fp->count)) {
		atomic_sub(fp->count, &u->scm_stat.nr_fds);
		unix_del_edges(fp);
	}
	...
}

unix_destruct_scm() -> unix_detach_fds() -> unix_destroy_fpl() releases the
files and the GC edges but never touches nr_fds.

The OOB drop branch a few lines below in this same function frees a skb that
may still carry UNIXCB(skb).fp without going through unix_orphan_scm():

	if (skb == u->oob_skb) {
		WRITE_ONCE(u->oob_skb, NULL);
		spin_unlock(&queue->lock);
		mutex_unlock(&u->iolock);

		kfree_skb_reason(skb, SKB_DROP_REASON_UNIX_SKIP_OOB);
		return -EAGAIN;
	}

manage_oob() does the same with a bare consume_skb(read_skb) /
kfree_skb_reason(unread_skb), and unix_stream_recv_urg() frees a zero-length
predecessor with consume_skb(read_skb).

unix_stream_recv_urg() is also what creates the zero-length-with-fds state:
it copies the OOB byte and bumps UNIXCB(oob_skb).consumed without detaching
fds, so the leftover skb can still hold SCM_RIGHTS charged earlier by
scm_stat_add() in queue_oob().

Should those paths also call unix_orphan_scm() so the nr_fds charge reported
as scm_fds: in /proc/<pid>/fdinfo does not stay permanently inflated?

>  	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;
> +	}
> +

[Severity: High]
Can this kfree_skb() discard unread stream bytes?

At this point in unix_stream_read_skb() the skb has already been unlinked by
__skb_dequeue(), and u->inq_len has already been reduced:

	WRITE_ONCE(u->inq_len, u->inq_len - unix_skb_len(skb));

so once the skb is freed the remaining unix_skb_len(skb) bytes are neither
recoverable nor still accounted for in the queue length.

The commit message describes pskb_pull() as the way to "safely handle both
linear and non-linear paged skbs", but it can still fail.  For a pull past
the linear head it goes pskb_pull() -> pskb_may_pull() ->
__pskb_pull_tail() -> pskb_expand_head():

net/core/skbuff.c:__pskb_pull_tail() {
	...
	if (eat > 0 || skb_cloned(skb)) {
		if (pskb_expand_head(skb, 0, eat > 0 ? eat + 128 : 0,
				     GFP_ATOMIC))
			return NULL;
	}
	...
}

unix_stream_sendmsg() produces exactly the skbs that reach that branch:

net/unix/af_unix.c:unix_stream_sendmsg() {
	...
		data_len = max_t(int, 0, size - SKB_MAX_HEAD(0));
		data_len = min_t(size_t, size, PAGE_ALIGN(data_len));
		skb = sock_alloc_send_pskb(sk, size - data_len, data_len, ...);
	...
}

MSG_SPLICE_PAGES skbs have skb_headlen() == 0 entirely, so any partial
recv() leaves UNIXCB(skb).consumed beyond the linear head and forces the
fallible path with a GFP_ATOMIC expansion of up to the full frag size.

The return value also does not appear to reach anyone who can retry:

net/core/skmsg.c:sk_psock_verdict_data_ready() {
	...
	copied = ops->read_skb(sk, sk_psock_verdict_recv);
	if (copied >= 0) {
		...
		sk_psock_data_ready(sk, psock);
	}
	...
}

The -ENOMEM is swallowed, sk_psock_data_ready() is skipped for data still
queued behind the dropped skb, and the sockmap consumer sees a hole in an
ordered byte stream with no error reported to userspace.

Would it be better to restore the state on failure, for example
__skb_queue_head() plus adding unix_skb_len(skb) back to u->inq_len, and let
the caller come back later, instead of freeing the skb?

>  	return recv_actor(sk, skb);
>  }
>

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260921054823.1822122-1-benquike%40gmail.com