ACK: [SRU][M/F][PATCH 0/2] CVE-2024-26923

[Stefan Bader]

On 30.05.24 00:01, Bethany Jamison wrote:
> [Impact]
> 
> af_unix: Fix garbage collector racing against connect()
> 
> Garbage collector does not take into account the risk of embryo getting
> enqueued during the garbage collection. If such embryo has a peer that
> carries SCM_RIGHTS, two consecutive passes of scan_children() may see a
> different set of children. Leading to an incorrectly elevated inflight
> count, and then a dangling pointer within the gc_inflight_list.
> 
> sockets are AF_UNIX/SOCK_STREAM
> S is an unconnected socket
> L is a listening in-flight socket bound to addr, not in fdtable
> V's fd will be passed via sendmsg(), gets inflight count bumped
> 
> connect(S, addr)	sendmsg(S, [V]); close(V)	__unix_gc()
> ----------------	-------------------------	-----------
> 
> NS = unix_create1()
> skb1 = sock_wmalloc(NS)
> L = unix_find_other(addr)
> unix_state_lock(L)
> unix_peer(S) = NS
> 			// V count=1 inflight=0
> 
>   			NS = unix_peer(S)
>   			skb2 = sock_alloc()
> 			skb_queue_tail(NS, skb2[V])
> 
> 			// V became in-flight
> 			// V count=2 inflight=1
> 
> 			close(V)
> 
> 			// V count=1 inflight=1
> 			// GC candidate condition met
> 
> 						for u in gc_inflight_list:
> 						  if (total_refs == inflight_refs)
> 						    add u to gc_candidates
> 
> 						// gc_candidates={L, V}
> 
> 						for u in gc_candidates:
> 						  scan_children(u, dec_inflight)
> 
> 						// embryo (skb1) was not
> 						// reachable from L yet, so V's
> 						// inflight remains unchanged
> __skb_queue_tail(L, skb1)
> unix_state_unlock(L)
> 						for u in gc_candidates:
> 						  if (u.inflight)
> 						    scan_children(u, inc_inflight_move_tail)
> 
> 						// V count=1 inflight=2 (!)
> 
> If there is a GC-candidate listening socket, lock/unlock its state. This
> makes GC wait until the end of any ongoing connect() to that socket. After
> flipping the lock, a possibly SCM-laden embryo is already enqueued. And if
> there is another embryo coming, it can not possibly carry SCM_RIGHTS. At
> this point, unix_inflight() can not happen because unix_gc_lock is already
> taken. Inflight graph remains unaffected.
> 
> [Fix]
> 
> Noble:	fixed via stable
> Mantic:	Clean cherry-pick from fix and prereq commit
> Jammy:	fixed via stable
> Focal:	Mantic patch applied cleanly
> Bionic:	fix sent to esm ML
> Xenial: fix sent to esm ML
> Trusty:	not going to be fixed by us
> 
> [Test Case]
> 
> Compile and boot tested
> 
> [Where problems could occur]
> 
> This fix affects those who use the AF_UNIX socket family, an issue with
> this fix would be visable to the user via decreased system performance.
> 
> Kuniyuki Iwashima (1):
>    af_unix: Do not use atomic ops for unix_sk(sk)->inflight.
> 
> Michal Luczaj (1):
>    af_unix: Fix garbage collector racing against connect()
> 
>   include/net/af_unix.h |  2 +-
>   net/unix/af_unix.c    |  4 ++--
>   net/unix/garbage.c    | 35 +++++++++++++++++++++++++----------
>   net/unix/scm.c        |  8 +++++---
>   4 files changed, 33 insertions(+), 16 deletions(-)
> 

Acked-by: Stefan Bader <stefan.bader at canonical.com>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_0xE8675DEECBEECEA3.asc
Type: application/pgp-keys
Size: 48643 bytes
Desc: OpenPGP public key
URL: <https://lists.ubuntu.com/archives/kernel-team/attachments/20240531/cdc2d383/attachment-0001.key>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://lists.ubuntu.com/archives/kernel-team/attachments/20240531/cdc2d383/attachment-0001.sig>