* [PATCH v1 net 0/4] af_unix: Fix two OOB issues.
@ 2025-06-18 4:34 Kuniyuki Iwashima
2025-06-18 4:34 ` [PATCH v1 net 1/4] af_unix: Don't leave consecutive consumed OOB skbs Kuniyuki Iwashima
` (4 more replies)
0 siblings, 5 replies; 7+ messages in thread
From: Kuniyuki Iwashima @ 2025-06-18 4:34 UTC (permalink / raw)
To: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni
Cc: Simon Horman, Kuniyuki Iwashima, Kuniyuki Iwashima, netdev
From: Kuniyuki Iwashima <kuniyu@google.com>
Recently, two issues are reported regarding MSG_OOB.
Patch 1 fixes issues that happen when multiple consumed OOB
skbs are placed consecutively in the recv queue.
Patch 2 fixes an inconsistent behaviour that close()ing a socket
with a consumed OOB skb at the head of the recv queue triggers
-ECONNRESET on the peer's recv().
Kuniyuki Iwashima (4):
af_unix: Don't leave consecutive consumed OOB skbs.
af_unix: Add test for consecutive consumed OOB.
af_unix: Don't set -ECONNRESET for consumed OOB skb.
selftest: af_unix: Add tests for -ECONNRESET.
net/unix/af_unix.c | 29 +++-
tools/testing/selftests/net/af_unix/msg_oob.c | 142 +++++++++++++++++-
2 files changed, 160 insertions(+), 11 deletions(-)
--
2.49.0
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v1 net 1/4] af_unix: Don't leave consecutive consumed OOB skbs.
2025-06-18 4:34 [PATCH v1 net 0/4] af_unix: Fix two OOB issues Kuniyuki Iwashima
@ 2025-06-18 4:34 ` Kuniyuki Iwashima
2025-06-18 4:34 ` [PATCH v1 net 2/4] af_unix: Add test for consecutive consumed OOB Kuniyuki Iwashima
` (3 subsequent siblings)
4 siblings, 0 replies; 7+ messages in thread
From: Kuniyuki Iwashima @ 2025-06-18 4:34 UTC (permalink / raw)
To: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni
Cc: Simon Horman, Kuniyuki Iwashima, Kuniyuki Iwashima, netdev,
Jann Horn
From: Kuniyuki Iwashima <kuniyu@google.com>
Jann Horn reported a use-after-free in unix_stream_read_generic().
The following sequences reproduce the issue:
$ python3
from socket import *
s1, s2 = socketpair(AF_UNIX, SOCK_STREAM)
s1.send(b'x', MSG_OOB)
s2.recv(1, MSG_OOB) # leave a consumed OOB skb
s1.send(b'y', MSG_OOB)
s2.recv(1, MSG_OOB) # leave a consumed OOB skb
s1.send(b'z', MSG_OOB)
s2.recv(1) # recv 'z' illegally
s2.recv(1, MSG_OOB) # access 'z' skb (use-after-free)
Even though a user reads OOB data, the skb holding the data stays on
the recv queue to mark the OOB boundary and break the next recv().
After the last send() in the scenario above, the sk2's recv queue has
2 leading consumed OOB skbs and 1 real OOB skb.
Then, the following happens during the next recv() without MSG_OOB
1. unix_stream_read_generic() peeks the first consumed OOB skb
2. manage_oob() returns the next consumed OOB skb
3. unix_stream_read_generic() fetches the next not-yet-consumed OOB skb
4. unix_stream_read_generic() reads and frees the OOB skb
, and the last recv(MSG_OOB) triggers KASAN splat.
The 3. above occurs because of the SO_PEEK_OFF code, which does not
expect unix_skb_len(skb) to be 0, but this is true for such consumed
OOB skbs.
while (skip >= unix_skb_len(skb)) {
skip -= unix_skb_len(skb);
skb = skb_peek_next(skb, &sk->sk_receive_queue);
...
}
In addition to this use-after-free, there is another issue that
ioctl(SIOCATMARK) does not function properly with consecutive consumed
OOB skbs.
So, nothing good comes out of such a situation.
Instead of complicating manage_oob(), ioctl() handling, and the next
ECONNRESET fix by introducing a loop for consecutive consumed OOB skbs,
let's not leave such consecutive OOB unnecessarily.
Now, while receiving an OOB skb in unix_stream_recv_urg(), if its
previous skb is a consumed OOB skb, it is freed.
[0]:
BUG: KASAN: slab-use-after-free in unix_stream_read_actor (net/unix/af_unix.c:3027)
Read of size 4 at addr ffff888106ef2904 by task python3/315
CPU: 2 UID: 0 PID: 315 Comm: python3 Not tainted 6.16.0-rc1-00407-gec315832f6f9 #8 PREEMPT(voluntary)
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.3-4.fc42 04/01/2014
Call Trace:
<TASK>
dump_stack_lvl (lib/dump_stack.c:122)
print_report (mm/kasan/report.c:409 mm/kasan/report.c:521)
kasan_report (mm/kasan/report.c:636)
unix_stream_read_actor (net/unix/af_unix.c:3027)
unix_stream_read_generic (net/unix/af_unix.c:2708 net/unix/af_unix.c:2847)
unix_stream_recvmsg (net/unix/af_unix.c:3048)
sock_recvmsg (net/socket.c:1063 (discriminator 20) net/socket.c:1085 (discriminator 20))
__sys_recvfrom (net/socket.c:2278)
__x64_sys_recvfrom (net/socket.c:2291 (discriminator 1) net/socket.c:2287 (discriminator 1) net/socket.c:2287 (discriminator 1))
do_syscall_64 (arch/x86/entry/syscall_64.c:63 (discriminator 1) arch/x86/entry/syscall_64.c:94 (discriminator 1))
entry_SYSCALL_64_after_hwframe (arch/x86/entry/entry_64.S:130)
RIP: 0033:0x7f8911fcea06
Code: 5d e8 41 8b 93 08 03 00 00 59 5e 48 83 f8 fc 75 19 83 e2 39 83 fa 08 75 11 e8 26 ff ff ff 66 0f 1f 44 00 00 48 8b 45 10 0f 05 <48> 8b 5d f8 c9 c3 0f 1f 40 00 f3 0f 1e fa 55 48 89 e5 48 83 ec 08
RSP: 002b:00007fffdb0dccb0 EFLAGS: 00000202 ORIG_RAX: 000000000000002d
RAX: ffffffffffffffda RBX: 00007fffdb0dcdc8 RCX: 00007f8911fcea06
RDX: 0000000000000001 RSI: 00007f8911a5e060 RDI: 0000000000000006
RBP: 00007fffdb0dccd0 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000001 R11: 0000000000000202 R12: 00007f89119a7d20
R13: ffffffffc4653600 R14: 0000000000000000 R15: 0000000000000000
</TASK>
Allocated by task 315:
kasan_save_stack (mm/kasan/common.c:48)
kasan_save_track (mm/kasan/common.c:60 (discriminator 1) mm/kasan/common.c:69 (discriminator 1))
__kasan_slab_alloc (mm/kasan/common.c:348)
kmem_cache_alloc_node_noprof (./include/linux/kasan.h:250 mm/slub.c:4148 mm/slub.c:4197 mm/slub.c:4249)
__alloc_skb (net/core/skbuff.c:660 (discriminator 4))
alloc_skb_with_frags (./include/linux/skbuff.h:1336 net/core/skbuff.c:6668)
sock_alloc_send_pskb (net/core/sock.c:2993)
unix_stream_sendmsg (./include/net/sock.h:1847 net/unix/af_unix.c:2256 net/unix/af_unix.c:2418)
__sys_sendto (net/socket.c:712 (discriminator 20) net/socket.c:727 (discriminator 20) net/socket.c:2226 (discriminator 20))
__x64_sys_sendto (net/socket.c:2233 (discriminator 1) net/socket.c:2229 (discriminator 1) net/socket.c:2229 (discriminator 1))
do_syscall_64 (arch/x86/entry/syscall_64.c:63 (discriminator 1) arch/x86/entry/syscall_64.c:94 (discriminator 1))
entry_SYSCALL_64_after_hwframe (arch/x86/entry/entry_64.S:130)
Freed by task 315:
kasan_save_stack (mm/kasan/common.c:48)
kasan_save_track (mm/kasan/common.c:60 (discriminator 1) mm/kasan/common.c:69 (discriminator 1))
kasan_save_free_info (mm/kasan/generic.c:579 (discriminator 1))
__kasan_slab_free (mm/kasan/common.c:271)
kmem_cache_free (mm/slub.c:4643 (discriminator 3) mm/slub.c:4745 (discriminator 3))
unix_stream_read_generic (net/unix/af_unix.c:3010)
unix_stream_recvmsg (net/unix/af_unix.c:3048)
sock_recvmsg (net/socket.c:1063 (discriminator 20) net/socket.c:1085 (discriminator 20))
__sys_recvfrom (net/socket.c:2278)
__x64_sys_recvfrom (net/socket.c:2291 (discriminator 1) net/socket.c:2287 (discriminator 1) net/socket.c:2287 (discriminator 1))
do_syscall_64 (arch/x86/entry/syscall_64.c:63 (discriminator 1) arch/x86/entry/syscall_64.c:94 (discriminator 1))
entry_SYSCALL_64_after_hwframe (arch/x86/entry/entry_64.S:130)
The buggy address belongs to the object at ffff888106ef28c0
which belongs to the cache skbuff_head_cache of size 224
The buggy address is located 68 bytes inside of
freed 224-byte region [ffff888106ef28c0, ffff888106ef29a0)
The buggy address belongs to the physical page:
page: refcount:0 mapcount:0 mapping:0000000000000000 index:0xffff888106ef3cc0 pfn:0x106ef2
head: order:1 mapcount:0 entire_mapcount:0 nr_pages_mapped:0 pincount:0
flags: 0x200000000000040(head|node=0|zone=2)
page_type: f5(slab)
raw: 0200000000000040 ffff8881001d28c0 ffffea000422fe00 0000000000000004
raw: ffff888106ef3cc0 0000000080190010 00000000f5000000 0000000000000000
head: 0200000000000040 ffff8881001d28c0 ffffea000422fe00 0000000000000004
head: ffff888106ef3cc0 0000000080190010 00000000f5000000 0000000000000000
head: 0200000000000001 ffffea00041bbc81 00000000ffffffff 00000000ffffffff
head: 0000000000000000 0000000000000000 00000000ffffffff 0000000000000000
page dumped because: kasan: bad access detected
Memory state around the buggy address:
ffff888106ef2800: 00 00 00 00 00 00 00 00 00 00 00 00 fc fc fc fc
ffff888106ef2880: fc fc fc fc fc fc fc fc fa fb fb fb fb fb fb fb
>ffff888106ef2900: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
^
ffff888106ef2980: fb fb fb fb fc fc fc fc fc fc fc fc fc fc fc fc
ffff888106ef2a00: fa fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
Fixes: 314001f0bf92 ("af_unix: Add OOB support")
Reported-by: Jann Horn <jannh@google.com>
Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
---
net/unix/af_unix.c | 13 +++++++++++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
index 22e170fb5dda..5392aa53cbc8 100644
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -2680,11 +2680,11 @@ struct unix_stream_read_state {
#if IS_ENABLED(CONFIG_AF_UNIX_OOB)
static int unix_stream_recv_urg(struct unix_stream_read_state *state)
{
+ struct sk_buff *oob_skb, *read_skb = NULL;
struct socket *sock = state->socket;
struct sock *sk = sock->sk;
struct unix_sock *u = unix_sk(sk);
int chunk = 1;
- struct sk_buff *oob_skb;
mutex_lock(&u->iolock);
unix_state_lock(sk);
@@ -2699,9 +2699,16 @@ static int unix_stream_recv_urg(struct unix_stream_read_state *state)
oob_skb = u->oob_skb;
- if (!(state->flags & MSG_PEEK))
+ if (!(state->flags & MSG_PEEK)) {
WRITE_ONCE(u->oob_skb, NULL);
+ if (oob_skb->prev != (struct sk_buff *)&sk->sk_receive_queue &&
+ !unix_skb_len(oob_skb->prev)) {
+ read_skb = oob_skb->prev;
+ __skb_unlink(read_skb, &sk->sk_receive_queue);
+ }
+ }
+
spin_unlock(&sk->sk_receive_queue.lock);
unix_state_unlock(sk);
@@ -2712,6 +2719,8 @@ static int unix_stream_recv_urg(struct unix_stream_read_state *state)
mutex_unlock(&u->iolock);
+ consume_skb(read_skb);
+
if (chunk < 0)
return -EFAULT;
--
2.49.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v1 net 2/4] af_unix: Add test for consecutive consumed OOB.
2025-06-18 4:34 [PATCH v1 net 0/4] af_unix: Fix two OOB issues Kuniyuki Iwashima
2025-06-18 4:34 ` [PATCH v1 net 1/4] af_unix: Don't leave consecutive consumed OOB skbs Kuniyuki Iwashima
@ 2025-06-18 4:34 ` Kuniyuki Iwashima
2025-06-18 4:34 ` [PATCH v1 net 3/4] af_unix: Don't set -ECONNRESET for consumed OOB skb Kuniyuki Iwashima
` (2 subsequent siblings)
4 siblings, 0 replies; 7+ messages in thread
From: Kuniyuki Iwashima @ 2025-06-18 4:34 UTC (permalink / raw)
To: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni
Cc: Simon Horman, Kuniyuki Iwashima, Kuniyuki Iwashima, netdev
From: Kuniyuki Iwashima <kuniyu@google.com>
Let's add a test case where consecutive concumed OOB skbs stay
at the head of the queue.
Without the previous patch, ioctl(SIOCATMARK) assertion fails.
Before:
# RUN msg_oob.no_peek.ex_oob_ex_oob_oob ...
# msg_oob.c:305:ex_oob_ex_oob_oob:Expected answ[0] (0) == oob_head (1)
# ex_oob_ex_oob_oob: Test terminated by assertion
# FAIL msg_oob.no_peek.ex_oob_ex_oob_oob
not ok 12 msg_oob.no_peek.ex_oob_ex_oob_oob
After:
# RUN msg_oob.no_peek.ex_oob_ex_oob_oob ...
# OK msg_oob.no_peek.ex_oob_ex_oob_oob
ok 12 msg_oob.no_peek.ex_oob_ex_oob_oob
Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
---
tools/testing/selftests/net/af_unix/msg_oob.c | 23 +++++++++++++++++++
1 file changed, 23 insertions(+)
diff --git a/tools/testing/selftests/net/af_unix/msg_oob.c b/tools/testing/selftests/net/af_unix/msg_oob.c
index 3ed3882a93b8..918509a3f040 100644
--- a/tools/testing/selftests/net/af_unix/msg_oob.c
+++ b/tools/testing/selftests/net/af_unix/msg_oob.c
@@ -548,6 +548,29 @@ TEST_F(msg_oob, ex_oob_oob)
siocatmarkpair(false);
}
+TEST_F(msg_oob, ex_oob_ex_oob_oob)
+{
+ sendpair("x", 1, MSG_OOB);
+ epollpair(true);
+ siocatmarkpair(true);
+
+ recvpair("x", 1, 1, MSG_OOB);
+ epollpair(false);
+ siocatmarkpair(true);
+
+ sendpair("y", 1, MSG_OOB);
+ epollpair(true);
+ siocatmarkpair(true);
+
+ recvpair("y", 1, 1, MSG_OOB);
+ epollpair(false);
+ siocatmarkpair(true);
+
+ sendpair("z", 1, MSG_OOB);
+ epollpair(true);
+ siocatmarkpair(true);
+}
+
TEST_F(msg_oob, ex_oob_ahead_break)
{
sendpair("hello", 5, MSG_OOB);
--
2.49.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v1 net 3/4] af_unix: Don't set -ECONNRESET for consumed OOB skb.
2025-06-18 4:34 [PATCH v1 net 0/4] af_unix: Fix two OOB issues Kuniyuki Iwashima
2025-06-18 4:34 ` [PATCH v1 net 1/4] af_unix: Don't leave consecutive consumed OOB skbs Kuniyuki Iwashima
2025-06-18 4:34 ` [PATCH v1 net 2/4] af_unix: Add test for consecutive consumed OOB Kuniyuki Iwashima
@ 2025-06-18 4:34 ` Kuniyuki Iwashima
2025-06-18 4:34 ` [PATCH v1 net 4/4] selftest: af_unix: Add tests for -ECONNRESET Kuniyuki Iwashima
2025-06-18 13:41 ` [PATCH v1 net 0/4] af_unix: Fix two OOB issues Jakub Kicinski
4 siblings, 0 replies; 7+ messages in thread
From: Kuniyuki Iwashima @ 2025-06-18 4:34 UTC (permalink / raw)
To: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni
Cc: Simon Horman, Kuniyuki Iwashima, Kuniyuki Iwashima, netdev,
Christian Brauner
From: Kuniyuki Iwashima <kuniyu@google.com>
Christian Brauner reported that even after MSG_OOB data is consumed,
calling close() on the receiver socket causes the peer's recv() to
return -ECONNRESET:
1. send() and recv() an OOB data.
>>> from socket import *
>>> s1, s2 = socketpair(AF_UNIX, SOCK_STREAM)
>>> s1.send(b'x', MSG_OOB)
1
>>> s2.recv(1, MSG_OOB)
b'x'
2. close() for s2 sets ECONNRESET to s1->sk_err even though
s2 consumed the OOB data
>>> s2.close()
>>> s1.recv(10, MSG_DONTWAIT)
...
ConnectionResetError: [Errno 104] Connection reset by peer
Even after being consumed, the skb holding the OOB 1-byte data stays in
the recv queue to mark the OOB boundary and break recv() at that point.
This must be considered while close()ing a socket.
Let's free the leading consumed OOB skb before checking the -ECONNRESET
condition in unix_release_sock().
Fixes: 314001f0bf92 ("af_unix: Add OOB support")
Reported-by: Christian Brauner <brauner@kernel.org>
Closes: https://lore.kernel.org/netdev/20250529-sinkt-abfeuern-e7b08200c6b0@brauner/
Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
---
net/unix/af_unix.c | 16 +++++++++++-----
1 file changed, 11 insertions(+), 5 deletions(-)
diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
index 5392aa53cbc8..50e56365f4f6 100644
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -660,6 +660,11 @@ static void unix_sock_destructor(struct sock *sk)
#endif
}
+static unsigned int unix_skb_len(const struct sk_buff *skb)
+{
+ return skb->len - UNIXCB(skb).consumed;
+}
+
static void unix_release_sock(struct sock *sk, int embrion)
{
struct unix_sock *u = unix_sk(sk);
@@ -687,6 +692,12 @@ static void unix_release_sock(struct sock *sk, int embrion)
unix_state_unlock(sk);
#if IS_ENABLED(CONFIG_AF_UNIX_OOB)
+ skb = skb_peek(&sk->sk_receive_queue);
+ if (skb && !unix_skb_len(skb)) {
+ __skb_unlink(skb, &sk->sk_receive_queue);
+ consume_skb(skb);
+ }
+
u->oob_skb = NULL;
#endif
@@ -2661,11 +2672,6 @@ static long unix_stream_data_wait(struct sock *sk, long timeo,
return timeo;
}
-static unsigned int unix_skb_len(const struct sk_buff *skb)
-{
- return skb->len - UNIXCB(skb).consumed;
-}
-
struct unix_stream_read_state {
int (*recv_actor)(struct sk_buff *, int, int,
struct unix_stream_read_state *);
--
2.49.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v1 net 4/4] selftest: af_unix: Add tests for -ECONNRESET.
2025-06-18 4:34 [PATCH v1 net 0/4] af_unix: Fix two OOB issues Kuniyuki Iwashima
` (2 preceding siblings ...)
2025-06-18 4:34 ` [PATCH v1 net 3/4] af_unix: Don't set -ECONNRESET for consumed OOB skb Kuniyuki Iwashima
@ 2025-06-18 4:34 ` Kuniyuki Iwashima
2025-06-18 13:41 ` [PATCH v1 net 0/4] af_unix: Fix two OOB issues Jakub Kicinski
4 siblings, 0 replies; 7+ messages in thread
From: Kuniyuki Iwashima @ 2025-06-18 4:34 UTC (permalink / raw)
To: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni
Cc: Simon Horman, Kuniyuki Iwashima, Kuniyuki Iwashima, netdev
From: Kuniyuki Iwashima <kuniyu@google.com>
A new function resetpair() calls close() for the receiver and checks
the return value from recv() on the initial sender side.
Now resetpair() is added to each test case and some additional test
cases.
Note that TCP sets -ECONNRESET to the consumed OOB, but we have decided
not to touch TCP MSG_OOB code in the past.
Before:
# RUN msg_oob.no_peek.ex_oob_ex_oob ...
# msg_oob.c:236:ex_oob_ex_oob:AF_UNIX :Connection reset by peer
# msg_oob.c:237:ex_oob_ex_oob:Expected:
# msg_oob.c:239:ex_oob_ex_oob:Expected ret[0] (-1) == expected_len (0)
# ex_oob_ex_oob: Test terminated by assertion
# FAIL msg_oob.no_peek.ex_oob_ex_oob
not ok 14 msg_oob.no_peek.ex_oob_ex_oob
...
# FAILED: 36 / 48 tests passed.
# Totals: pass:36 fail:12 xfail:0 xpass:0 skip:0 error:0
After:
# RUN msg_oob.no_peek.ex_oob_ex_oob ...
# msg_oob.c:244:ex_oob_ex_oob:AF_UNIX :
# msg_oob.c:245:ex_oob_ex_oob:TCP :Connection reset by peer
# OK msg_oob.no_peek.ex_oob_ex_oob
ok 14 msg_oob.no_peek.ex_oob_ex_oob
...
# PASSED: 48 / 48 tests passed.
# Totals: pass:48 fail:0 xfail:0 xpass:0 skip:0 error:0
Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
---
tools/testing/selftests/net/af_unix/msg_oob.c | 119 +++++++++++++++++-
1 file changed, 115 insertions(+), 4 deletions(-)
diff --git a/tools/testing/selftests/net/af_unix/msg_oob.c b/tools/testing/selftests/net/af_unix/msg_oob.c
index 918509a3f040..b5f474969917 100644
--- a/tools/testing/selftests/net/af_unix/msg_oob.c
+++ b/tools/testing/selftests/net/af_unix/msg_oob.c
@@ -210,7 +210,7 @@ static void __sendpair(struct __test_metadata *_metadata,
static void __recvpair(struct __test_metadata *_metadata,
FIXTURE_DATA(msg_oob) *self,
const char *expected_buf, int expected_len,
- int buf_len, int flags)
+ int buf_len, int flags, bool is_sender)
{
int i, ret[2], recv_errno[2], expected_errno = 0;
char recv_buf[2][BUF_SZ] = {};
@@ -221,7 +221,9 @@ static void __recvpair(struct __test_metadata *_metadata,
errno = 0;
for (i = 0; i < 2; i++) {
- ret[i] = recv(self->fd[i * 2 + 1], recv_buf[i], buf_len, flags);
+ int index = is_sender ? i * 2 : i * 2 + 1;
+
+ ret[i] = recv(self->fd[index], recv_buf[i], buf_len, flags);
recv_errno[i] = errno;
}
@@ -308,6 +310,20 @@ static void __siocatmarkpair(struct __test_metadata *_metadata,
ASSERT_EQ(answ[0], answ[1]);
}
+static void __resetpair(struct __test_metadata *_metadata,
+ FIXTURE_DATA(msg_oob) *self,
+ const FIXTURE_VARIANT(msg_oob) *variant,
+ bool reset)
+{
+ int i;
+
+ for (i = 0; i < 2; i++)
+ close(self->fd[i * 2 + 1]);
+
+ __recvpair(_metadata, self, "", reset ? -ECONNRESET : 0, 1,
+ variant->peek ? MSG_PEEK : 0, true);
+}
+
#define sendpair(buf, len, flags) \
__sendpair(_metadata, self, buf, len, flags)
@@ -316,9 +332,10 @@ static void __siocatmarkpair(struct __test_metadata *_metadata,
if (variant->peek) \
__recvpair(_metadata, self, \
expected_buf, expected_len, \
- buf_len, (flags) | MSG_PEEK); \
+ buf_len, (flags) | MSG_PEEK, false); \
__recvpair(_metadata, self, \
- expected_buf, expected_len, buf_len, flags); \
+ expected_buf, expected_len, \
+ buf_len, flags, false); \
} while (0)
#define epollpair(oob_remaining) \
@@ -330,6 +347,9 @@ static void __siocatmarkpair(struct __test_metadata *_metadata,
#define setinlinepair() \
__setinlinepair(_metadata, self)
+#define resetpair(reset) \
+ __resetpair(_metadata, self, variant, reset)
+
#define tcp_incompliant \
for (self->tcp_compliant = false; \
self->tcp_compliant == false; \
@@ -344,6 +364,21 @@ TEST_F(msg_oob, non_oob)
recvpair("", -EINVAL, 1, MSG_OOB);
epollpair(false);
siocatmarkpair(false);
+
+ resetpair(true);
+}
+
+TEST_F(msg_oob, non_oob_no_reset)
+{
+ sendpair("x", 1, 0);
+ epollpair(false);
+ siocatmarkpair(false);
+
+ recvpair("x", 1, 1, 0);
+ epollpair(false);
+ siocatmarkpair(false);
+
+ resetpair(false);
}
TEST_F(msg_oob, oob)
@@ -355,6 +390,19 @@ TEST_F(msg_oob, oob)
recvpair("x", 1, 1, MSG_OOB);
epollpair(false);
siocatmarkpair(true);
+
+ tcp_incompliant {
+ resetpair(false); /* TCP sets -ECONNRESET for ex-OOB. */
+ }
+}
+
+TEST_F(msg_oob, oob_reset)
+{
+ sendpair("x", 1, MSG_OOB);
+ epollpair(true);
+ siocatmarkpair(true);
+
+ resetpair(true);
}
TEST_F(msg_oob, oob_drop)
@@ -370,6 +418,8 @@ TEST_F(msg_oob, oob_drop)
recvpair("", -EINVAL, 1, MSG_OOB);
epollpair(false);
siocatmarkpair(false);
+
+ resetpair(false);
}
TEST_F(msg_oob, oob_ahead)
@@ -385,6 +435,10 @@ TEST_F(msg_oob, oob_ahead)
recvpair("hell", 4, 4, 0);
epollpair(false);
siocatmarkpair(true);
+
+ tcp_incompliant {
+ resetpair(false); /* TCP sets -ECONNRESET for ex-OOB. */
+ }
}
TEST_F(msg_oob, oob_break)
@@ -403,6 +457,8 @@ TEST_F(msg_oob, oob_break)
recvpair("", -EAGAIN, 1, 0);
siocatmarkpair(false);
+
+ resetpair(false);
}
TEST_F(msg_oob, oob_ahead_break)
@@ -426,6 +482,8 @@ TEST_F(msg_oob, oob_ahead_break)
recvpair("world", 5, 5, 0);
epollpair(false);
siocatmarkpair(false);
+
+ resetpair(false);
}
TEST_F(msg_oob, oob_break_drop)
@@ -449,6 +507,8 @@ TEST_F(msg_oob, oob_break_drop)
recvpair("", -EINVAL, 1, MSG_OOB);
epollpair(false);
siocatmarkpair(false);
+
+ resetpair(false);
}
TEST_F(msg_oob, ex_oob_break)
@@ -476,6 +536,8 @@ TEST_F(msg_oob, ex_oob_break)
recvpair("ld", 2, 2, 0);
epollpair(false);
siocatmarkpair(false);
+
+ resetpair(false);
}
TEST_F(msg_oob, ex_oob_drop)
@@ -498,6 +560,8 @@ TEST_F(msg_oob, ex_oob_drop)
epollpair(false);
siocatmarkpair(true);
}
+
+ resetpair(false);
}
TEST_F(msg_oob, ex_oob_drop_2)
@@ -523,6 +587,8 @@ TEST_F(msg_oob, ex_oob_drop_2)
epollpair(false);
siocatmarkpair(true);
}
+
+ resetpair(false);
}
TEST_F(msg_oob, ex_oob_oob)
@@ -546,6 +612,31 @@ TEST_F(msg_oob, ex_oob_oob)
recvpair("", -EINVAL, 1, MSG_OOB);
epollpair(false);
siocatmarkpair(false);
+
+ resetpair(false);
+}
+
+TEST_F(msg_oob, ex_oob_ex_oob)
+{
+ sendpair("x", 1, MSG_OOB);
+ epollpair(true);
+ siocatmarkpair(true);
+
+ recvpair("x", 1, 1, MSG_OOB);
+ epollpair(false);
+ siocatmarkpair(true);
+
+ sendpair("y", 1, MSG_OOB);
+ epollpair(true);
+ siocatmarkpair(true);
+
+ recvpair("y", 1, 1, MSG_OOB);
+ epollpair(false);
+ siocatmarkpair(true);
+
+ tcp_incompliant {
+ resetpair(false); /* TCP sets -ECONNRESET for ex-OOB. */
+ }
}
TEST_F(msg_oob, ex_oob_ex_oob_oob)
@@ -599,6 +690,10 @@ TEST_F(msg_oob, ex_oob_ahead_break)
recvpair("d", 1, 1, MSG_OOB);
epollpair(false);
siocatmarkpair(true);
+
+ tcp_incompliant {
+ resetpair(false); /* TCP sets -ECONNRESET for ex-OOB. */
+ }
}
TEST_F(msg_oob, ex_oob_siocatmark)
@@ -618,6 +713,8 @@ TEST_F(msg_oob, ex_oob_siocatmark)
recvpair("hell", 4, 4, 0); /* Intentionally stop at ex-OOB. */
epollpair(true);
siocatmarkpair(false);
+
+ resetpair(true);
}
TEST_F(msg_oob, inline_oob)
@@ -635,6 +732,8 @@ TEST_F(msg_oob, inline_oob)
recvpair("x", 1, 1, 0);
epollpair(false);
siocatmarkpair(false);
+
+ resetpair(false);
}
TEST_F(msg_oob, inline_oob_break)
@@ -656,6 +755,8 @@ TEST_F(msg_oob, inline_oob_break)
recvpair("o", 1, 1, 0);
epollpair(false);
siocatmarkpair(false);
+
+ resetpair(false);
}
TEST_F(msg_oob, inline_oob_ahead_break)
@@ -684,6 +785,8 @@ TEST_F(msg_oob, inline_oob_ahead_break)
epollpair(false);
siocatmarkpair(false);
+
+ resetpair(false);
}
TEST_F(msg_oob, inline_ex_oob_break)
@@ -709,6 +812,8 @@ TEST_F(msg_oob, inline_ex_oob_break)
recvpair("rld", 3, 3, 0);
epollpair(false);
siocatmarkpair(false);
+
+ resetpair(false);
}
TEST_F(msg_oob, inline_ex_oob_no_drop)
@@ -730,6 +835,8 @@ TEST_F(msg_oob, inline_ex_oob_no_drop)
recvpair("y", 1, 1, 0);
epollpair(false);
siocatmarkpair(false);
+
+ resetpair(false);
}
TEST_F(msg_oob, inline_ex_oob_drop)
@@ -754,6 +861,8 @@ TEST_F(msg_oob, inline_ex_oob_drop)
epollpair(false);
siocatmarkpair(false);
}
+
+ resetpair(false);
}
TEST_F(msg_oob, inline_ex_oob_siocatmark)
@@ -775,6 +884,8 @@ TEST_F(msg_oob, inline_ex_oob_siocatmark)
recvpair("hell", 4, 4, 0); /* Intentionally stop at ex-OOB. */
epollpair(true);
siocatmarkpair(false);
+
+ resetpair(true);
}
TEST_HARNESS_MAIN
--
2.49.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v1 net 0/4] af_unix: Fix two OOB issues.
2025-06-18 4:34 [PATCH v1 net 0/4] af_unix: Fix two OOB issues Kuniyuki Iwashima
` (3 preceding siblings ...)
2025-06-18 4:34 ` [PATCH v1 net 4/4] selftest: af_unix: Add tests for -ECONNRESET Kuniyuki Iwashima
@ 2025-06-18 13:41 ` Jakub Kicinski
2025-06-18 16:28 ` Kuniyuki Iwashima
4 siblings, 1 reply; 7+ messages in thread
From: Jakub Kicinski @ 2025-06-18 13:41 UTC (permalink / raw)
To: Kuniyuki Iwashima
Cc: David S. Miller, Eric Dumazet, Paolo Abeni, Simon Horman,
Kuniyuki Iwashima, netdev
On Tue, 17 Jun 2025 21:34:38 -0700 Kuniyuki Iwashima wrote:
> Patch 1 fixes issues that happen when multiple consumed OOB
> skbs are placed consecutively in the recv queue.
>
> Patch 2 fixes an inconsistent behaviour that close()ing a socket
> with a consumed OOB skb at the head of the recv queue triggers
> -ECONNRESET on the peer's recv().
It appears to break the scm_rights tests, including a UAF.
# # RUN scm_rights.stream_listener.self_ref ...
# # scm_rights.c:176:self_ref:Expected 0 (0) == ret (4)
# # self_ref: Test terminated by assertion
# # FAIL scm_rights.stream_listener.self_ref
# not ok 25 scm_rights.stream_listener.self_ref
# # RUN scm_rights.stream_listener.triangle ...
# # scm_rights.c:176:triangle:Expected 0 (0) == ret (12)
# # triangle: Test terminated by assertion
# # FAIL scm_rights.stream_listener.triangle
# not ok 26 scm_rights.stream_listener.triangle
# # RUN scm_rights.stream_listener.cross_edge ...
# # scm_rights.c:176:cross_edge:Expected 0 (0) == ret (16)
# # cross_edge: Test terminated by assertion
# # FAIL scm_rights.stream_listener.cross_edge
# not ok 27 scm_rights.stream_listener.cross_edge
# # RUN scm_rights.stream_listener.backtrack_from_scc ...
[ 5716.340166][T26625] ==================================================================
[ 5716.340494][T26625] BUG: KASAN: slab-use-after-free in __unix_walk_scc+0x8e0/0xce0
[ 5716.340761][T26625] Read of size 8 at addr ffff88801d8c6fd0 by task kworker/u17:0/26625
[ 5716.341015][T26625]
[ 5716.341103][T26625] CPU: 2 UID: 0 PID: 26625 Comm: kworker/u17:0 Not tainted 6.16.0-rc1-virtme #1 PREEMPT(full)
[ 5716.341109][T26625] Hardware name: Bochs Bochs, BIOS Bochs 01/01/2011
[ 5716.341112][T26625] Workqueue: events_unbound __unix_gc
[ 5716.341118][T26625] Call Trace:
[ 5716.341120][T26625] <TASK>
[ 5716.341123][T26625] dump_stack_lvl+0x82/0xd0
[ 5716.341133][T26625] print_address_description.constprop.0+0x2c/0x400
[ 5716.341141][T26625] ? __unix_walk_scc+0x8e0/0xce0
[ 5716.341145][T26625] print_report+0xb4/0x270
[ 5716.341148][T26625] ? __unix_walk_scc+0x8e0/0xce0
[ 5716.341152][T26625] ? kasan_addr_to_slab+0x25/0x80
[ 5716.341155][T26625] ? __unix_walk_scc+0x8e0/0xce0
[ 5716.341158][T26625] kasan_report+0xca/0x100
[ 5716.341163][T26625] ? __unix_walk_scc+0x8e0/0xce0
[ 5716.341168][T26625] __unix_walk_scc+0x8e0/0xce0
[ 5716.341174][T26625] ? __pfx___unix_walk_scc+0x10/0x10
[ 5716.341178][T26625] ? do_raw_spin_lock+0x130/0x270
[ 5716.341185][T26625] ? __pfx_do_raw_spin_lock+0x10/0x10
[ 5716.341189][T26625] ? lock_acquire+0x10c/0x170
[ 5716.341192][T26625] ? __unix_gc+0x8b/0x400
[ 5716.341197][T26625] __unix_gc+0x29f/0x400
[ 5716.341201][T26625] ? __pfx___unix_gc+0x10/0x10
[ 5716.341207][T26625] ? rcu_is_watching+0x12/0xc0
[ 5716.341215][T26625] ? rcu_is_watching+0x12/0xc0
[ 5716.341219][T26625] process_one_work+0xe43/0x1660
[ 5716.341228][T26625] ? __pfx_process_one_work+0x10/0x10
[ 5716.341233][T26625] ? assign_work+0x16c/0x240
[ 5716.341241][T26625] worker_thread+0x591/0xcf0
[ 5716.341246][T26625] ? __pfx_worker_thread+0x10/0x10
[ 5716.341250][T26625] kthread+0x37e/0x600
[ 5716.341254][T26625] ? __pfx_kthread+0x10/0x10
[ 5716.341256][T26625] ? ret_from_fork+0x1b/0x320
[ 5716.341261][T26625] ? __lock_release+0x5d/0x170
[ 5716.341265][T26625] ? rcu_is_watching+0x12/0xc0
[ 5716.341268][T26625] ? __pfx_kthread+0x10/0x10
[ 5716.341271][T26625] ret_from_fork+0x240/0x320
[ 5716.341274][T26625] ? __pfx_kthread+0x10/0x10
[ 5716.341276][T26625] ret_from_fork_asm+0x1a/0x30
[ 5716.341286][T26625] </TASK>
[ 5716.341288][T26625]
[ 5716.347648][T26625] Allocated by task 12654:
[ 5716.347814][T26625] kasan_save_stack+0x24/0x50
[ 5716.347983][T26625] kasan_save_track+0x14/0x30
[ 5716.348171][T26625] __kasan_slab_alloc+0x59/0x70
[ 5716.348348][T26625] kmem_cache_alloc_noprof+0x10b/0x330
[ 5716.348522][T26625] sk_prot_alloc.constprop.0+0x4e/0x1b0
[ 5716.348695][T26625] sk_alloc+0x36/0x6c0
[ 5716.348823][T26625] unix_create1+0x84/0x6f0
[ 5716.348991][T26625] unix_create+0xcb/0x170
[ 5716.349119][T26625] __sock_create+0x23c/0x6a0
[ 5716.349287][T26625] __sys_socket+0x11a/0x1d0
[ 5716.349457][T26625] __x64_sys_socket+0x72/0xb0
[ 5716.349634][T26625] do_syscall_64+0xc1/0x380
[ 5716.349803][T26625] entry_SYSCALL_64_after_hwframe+0x77/0x7f
[ 5716.350014][T26625]
[ 5716.350098][T26625] Freed by task 12654:
[ 5716.350223][T26625] kasan_save_stack+0x24/0x50
[ 5716.350390][T26625] kasan_save_track+0x14/0x30
[ 5716.350586][T26625] kasan_save_free_info+0x3b/0x60
[ 5716.350759][T26625] __kasan_slab_free+0x38/0x50
[ 5716.350930][T26625] kmem_cache_free+0x149/0x330
[ 5716.351099][T26625] __sk_destruct+0x46e/0x780
[ 5716.351269][T26625] unix_release_sock+0xa0e/0xf90
[ 5716.351440][T26625] unix_release+0x8c/0xf0
[ 5716.351574][T26625] __sock_release+0xa6/0x260
[ 5716.351763][T26625] sock_close+0x18/0x20
[ 5716.351980][T26625] __fput+0x35c/0xa80
[ 5716.352125][T26625] fput_close_sync+0xdd/0x190
[ 5716.352293][T26625] __x64_sys_close+0x7d/0xd0
[ 5716.352464][T26625] do_syscall_64+0xc1/0x380
[ 5716.352724][T26625] entry_SYSCALL_64_after_hwframe+0x77/0x7f
[ 5716.352933][T26625]
[ 5716.353018][T26625] The buggy address belongs to the object at ffff88801d8c6940
[ 5716.353018][T26625] which belongs to the cache UNIX-STREAM of size 1984
[ 5716.353553][T26625] The buggy address is located 1680 bytes inside of
[ 5716.353553][T26625] freed 1984-byte region [ffff88801d8c6940, ffff88801d8c7100)
[ 5716.353951][T26625]
[ 5716.354037][T26625] The buggy address belongs to the physical page:
[ 5716.354324][T26625] page: refcount:0 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0x1d8c0
[ 5716.354621][T26625] head: order:3 mapcount:0 entire_mapcount:0 nr_pages_mapped:0 pincount:0
[ 5716.354966][T26625] flags: 0x80000000000040(head|node=0|zone=1)
[ 5716.355181][T26625] page_type: f5(slab)
[ 5716.355311][T26625] raw: 0080000000000040 ffff888005b4edc0 ffffea00002b7610 ffffea0000763210
[ 5716.355702][T26625] raw: 0000000000000000 00000000000e000e 00000000f5000000 0000000000000000
[ 5716.356022][T26625] head: 0080000000000040 ffff888005b4edc0 ffffea00002b7610 ffffea0000763210
[ 5716.356330][T26625] head: 0000000000000000 00000000000e000e 00000000f5000000 0000000000000000
[ 5716.356727][T26625] head: 0080000000000003 ffffea0000763001 00000000ffffffff 00000000ffffffff
[ 5716.357027][T26625] head: 0000000000000000 0000000000000000 00000000ffffffff 0000000000000000
[ 5716.357434][T26625] page dumped because: kasan: bad access detected
[ 5716.357635][T26625]
[ 5716.357716][T26625] Memory state around the buggy address:
[ 5716.357874][T26625] ffff88801d8c6e80: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
[ 5716.358115][T26625] ffff88801d8c6f00: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
[ 5716.358382][T26625] >ffff88801d8c6f80: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
[ 5716.358720][T26625] ^
[ 5716.358921][T26625] ffff88801d8c7000: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
[ 5716.359162][T26625] ffff88801d8c7080: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
[ 5716.359506][T26625] ==================================================================
[ 5716.359805][T26625] Disabling lock debugging due to kernel taint
# # scm_rights.c:176:backtrack_from_scc:Expected 0 (0) == ret (22)
# # backtrack_from_scc: Test terminated by assertion
# # FAIL scm_rights.stream_listener.backtrack_from_scc
# not ok 28 scm_rights.stream_listener.backtrack_from_scc
--
pw-bot: cr
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v1 net 0/4] af_unix: Fix two OOB issues.
2025-06-18 13:41 ` [PATCH v1 net 0/4] af_unix: Fix two OOB issues Jakub Kicinski
@ 2025-06-18 16:28 ` Kuniyuki Iwashima
0 siblings, 0 replies; 7+ messages in thread
From: Kuniyuki Iwashima @ 2025-06-18 16:28 UTC (permalink / raw)
To: kuba; +Cc: davem, edumazet, horms, kuni1840, kuniyu, netdev, pabeni
From: Jakub Kicinski <kuba@kernel.org>
Date: Wed, 18 Jun 2025 06:41:26 -0700
> On Tue, 17 Jun 2025 21:34:38 -0700 Kuniyuki Iwashima wrote:
> > Patch 1 fixes issues that happen when multiple consumed OOB
> > skbs are placed consecutively in the recv queue.
> >
> > Patch 2 fixes an inconsistent behaviour that close()ing a socket
> > with a consumed OOB skb at the head of the recv queue triggers
> > -ECONNRESET on the peer's recv().
>
> It appears to break the scm_rights tests, including a UAF.
Sorry, I forgot the length of a skb holding embryo is 0, maybe
sock_omalloc(1, ...) confused me.
Will fix it in v2.
Thanks!
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2025-06-18 16:28 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-06-18 4:34 [PATCH v1 net 0/4] af_unix: Fix two OOB issues Kuniyuki Iwashima
2025-06-18 4:34 ` [PATCH v1 net 1/4] af_unix: Don't leave consecutive consumed OOB skbs Kuniyuki Iwashima
2025-06-18 4:34 ` [PATCH v1 net 2/4] af_unix: Add test for consecutive consumed OOB Kuniyuki Iwashima
2025-06-18 4:34 ` [PATCH v1 net 3/4] af_unix: Don't set -ECONNRESET for consumed OOB skb Kuniyuki Iwashima
2025-06-18 4:34 ` [PATCH v1 net 4/4] selftest: af_unix: Add tests for -ECONNRESET Kuniyuki Iwashima
2025-06-18 13:41 ` [PATCH v1 net 0/4] af_unix: Fix two OOB issues Jakub Kicinski
2025-06-18 16:28 ` Kuniyuki Iwashima
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).