lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Message-ID: <20240507163653.1131444-1-mhal@rbox.co>
Date: Tue,  7 May 2024 18:29:33 +0200
From: Michal Luczaj <mhal@...x.co>
To: netdev@...r.kernel.org
Cc: davem@...emloft.net,
	edumazet@...gle.com,
	kuba@...nel.org,
	pabeni@...hat.com,
	kuniyu@...zon.com,
	Michal Luczaj <mhal@...x.co>
Subject: [PATCH net-next] af_unix: Fix garbage collector racing against send() MSG_OOB

Garbage collector takes care to explicitly drop oob_skb references before
purging the hit list. However, it does not account for a race: another
oob_skb might get assigned through in-flight socket's peer.

a/A and b/B are peers; A and B have been close()d and form an (OOB)
in-flight cycle:

        a -- A <-> B -- b

When A's and B's queues are about to be collected, their peers are still
reachable. User can send another OOB packet:

queue_oob                       unix_gc
---------                       -------

unix_state_lock(u)
if (u->oob_sbk)
  consume_skb(u->oob_skb)
  // oob_skb refcnt == 1
				skb_queue_splice(u->queue, hitlist)
				if (u->oob_skb) {
				  kfree_skb(u->oob_skb)
				  // oob_skb refcnt == 0, but in hitlist (!)
				  u->oob_skb = NULL
				}
u->oob_skb = skb
skb_queue(u->queue, skb)
unix_state_unlock(u)

Tell GC to remove oob_skb under unix_state_lock().

Fixes: 4090fa373f0e ("af_unix: Replace garbage collection algorithm.")
Signed-off-by: Michal Luczaj <mhal@...x.co>
---
[   10.959053] refcount_t: underflow; use-after-free.
[   10.959072] WARNING: CPU: 0 PID: 711 at lib/refcount.c:28 refcount_warn_saturate+0xba/0x110
[   10.959077] Modules linked in: 9p netfs kvm_intel kvm 9pnet_virtio 9pnet i2c_piix4 zram crct10dif_pclmul crc32_pclmul crc32c_intel virtio_blk ghash_clmulni_intel serio_raw fuse qemu_fw_cfg virtio_console
[   10.959092] CPU: 0 PID: 711 Comm: kworker/0:3 Not tainted 6.9.0-rc6nokasan+ #7
[   10.959094] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Arch Linux 1.16.3-1-1 04/01/2014
[   10.959095] Workqueue: events delayed_fput
[   10.959098] RIP: 0010:refcount_warn_saturate+0xba/0x110
[   10.959100] Code: 01 01 e8 79 d7 86 ff 0f 0b c3 cc cc cc cc 80 3d a7 e1 cd 01 00 75 85 48 c7 c7 d0 c0 a2 82 c6 05 97 e1 cd 01 01 e8 56 d7 86 ff <0f> 0b c3 cc cc cc cc 80 3d 85 e1 cd 01 00 0f 85 5e ff ff ff 48 c7
[   10.959102] RSP: 0018:ffffc90000477ce8 EFLAGS: 00010296
[   10.959104] RAX: 0000000000000026 RBX: ffff88812d102440 RCX: 0000000000000000
[   10.959105] RDX: 0000000000000002 RSI: 0000000000000027 RDI: 00000000ffffffff
[   10.959106] RBP: 0000000000000300 R08: 0000000000000000 R09: 0000000000000003
[   10.959107] R10: ffffc90000477ba0 R11: ffffffff833591e8 R12: ffff88812d1026dc
[   10.959108] R13: ffff88812d102c68 R14: 0000000000000001 R15: ffff88812d102ac0
[   10.959109] FS:  0000000000000000(0000) GS:ffff88842fa00000(0000) knlGS:0000000000000000
[   10.959110] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[   10.959111] CR2: 00007f158e949900 CR3: 0000000003238000 CR4: 0000000000750ef0
[   10.959114] PKRU: 55555554
[   10.959115] Call Trace:
[   10.959116]  <TASK>
[   10.959118]  ? __warn+0x88/0x180
[   10.959121]  ? refcount_warn_saturate+0xba/0x110
[   10.959123]  ? report_bug+0x18d/0x1c0
[   10.959128]  ? handle_bug+0x3c/0x80
[   10.959130]  ? exc_invalid_op+0x13/0x60
[   10.959132]  ? asm_exc_invalid_op+0x16/0x20
[   10.959137]  ? refcount_warn_saturate+0xba/0x110
[   10.959139]  ? refcount_warn_saturate+0xba/0x110
[   10.959141]  sock_wfree+0xcf/0x300
[   10.959144]  unix_destruct_scm+0x77/0x90
[   10.959147]  skb_release_head_state+0x20/0x60
[   10.959149]  kfree_skb_reason+0x53/0x1e0
[   10.959152]  unix_release_sock+0x255/0x560
[   10.959155]  unix_release+0x2e/0x40
[   10.959156]  __sock_release+0x3a/0xc0
[   10.959159]  sock_close+0x14/0x20
[   10.959161]  __fput+0x9a/0x2d0
[   10.959163]  delayed_fput+0x1f/0x30
[   10.959165]  process_one_work+0x217/0x700
[   10.959167]  ? move_linked_works+0x70/0xa0
[   10.959170]  worker_thread+0x1ca/0x3b0
[   10.959173]  ? __pfx_worker_thread+0x10/0x10
[   10.959175]  kthread+0xdd/0x110
[   10.959176]  ? __pfx_kthread+0x10/0x10
[   10.959178]  ret_from_fork+0x2d/0x50
[   10.959181]  ? __pfx_kthread+0x10/0x10
[   10.959182]  ret_from_fork_asm+0x1a/0x30
[   10.959187]  </TASK>
[   10.959188] irq event stamp: 47083
[   10.959189] hardirqs last  enabled at (47089): [<ffffffff811c5a5d>] console_unlock+0xfd/0x130
[   10.959192] hardirqs last disabled at (47094): [<ffffffff811c5a42>] console_unlock+0xe2/0x130
[   10.959193] softirqs last  enabled at (46644): [<ffffffff81f1ed8f>] unix_release_sock+0x12f/0x560
[   10.959194] softirqs last disabled at (46642): [<ffffffff81f1ed6b>] unix_release_sock+0x10b/0x560

 include/net/af_unix.h |  1 +
 net/unix/garbage.c    | 10 ++++++----
 2 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/include/net/af_unix.h b/include/net/af_unix.h
index b6eedf7650da..6791c660b5ca 100644
--- a/include/net/af_unix.h
+++ b/include/net/af_unix.h
@@ -103,6 +103,7 @@ enum unix_socket_lock_class {
 	U_LOCK_GC_LISTENER, /* used for listening socket while determining gc
 			     * candidates to close a small race window.
 			     */
+	U_LOCK_GC_OOB, /* freeing oob_skb during GC. */
 };
 
 static inline void unix_state_lock_nested(struct sock *sk,
diff --git a/net/unix/garbage.c b/net/unix/garbage.c
index d76450133e4f..f2098653aef8 100644
--- a/net/unix/garbage.c
+++ b/net/unix/garbage.c
@@ -357,11 +357,10 @@ static void unix_collect_skb(struct list_head *scc, struct sk_buff_head *hitlist
 		u = edge->predecessor;
 		queue = &u->sk.sk_receive_queue;
 
-		spin_lock(&queue->lock);
-
 		if (u->sk.sk_state == TCP_LISTEN) {
 			struct sk_buff *skb;
 
+			spin_lock(&queue->lock);
 			skb_queue_walk(queue, skb) {
 				struct sk_buff_head *embryo_queue = &skb->sk->sk_receive_queue;
 
@@ -370,18 +369,21 @@ static void unix_collect_skb(struct list_head *scc, struct sk_buff_head *hitlist
 				skb_queue_splice_init(embryo_queue, hitlist);
 				spin_unlock(&embryo_queue->lock);
 			}
+			spin_unlock(&queue->lock);
 		} else {
+			spin_lock(&queue->lock);
 			skb_queue_splice_init(queue, hitlist);
+			spin_unlock(&queue->lock);
 
 #if IS_ENABLED(CONFIG_AF_UNIX_OOB)
+			unix_state_lock_nested(&u->sk, U_LOCK_GC_OOB);
 			if (u->oob_skb) {
 				kfree_skb(u->oob_skb);
 				u->oob_skb = NULL;
 			}
+			unix_state_unlock(&u->sk);
 #endif
 		}
-
-		spin_unlock(&queue->lock);
 	}
 }
 
-- 
2.45.0


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ