Re: [PATCH net 0/2] net: restore acquire ordering for two remaining

[Hang Nan <2122295973@qq.com>]

[-- Attachment #1: Type: text/plain, Size: 3883 bytes --]

Hi,

Thanks for the automated review.  Here is the missing information for the
series, plus the instruction-level evidence that was asked for.

How it was found: manual code inspection, not a runtime report.  While
looking at net/ lockless lookups right after 5b5d8f351dd9 landed, I
enumerated every refcount_inc_not_zero() call site in net/ (102 of them
today) and classified each one by whether the reference is taken only to
keep the object alive, or whether object state is re-validated after the
acquisition.  mptcp_token_get_sock() and mptcp_token_iter_next() are in
the second group: they re-read msk->token and sock_net(sk) after the
increment, and mptcp_token_destroy() unlinks the socket and then does
WRITE_ONCE(msk->token, 0), so the re-validation is the only thing that
rejects a socket whose token has already been cleared.
reuseport_migrate_sock() in 2/2 is the only other site with that shape
in net/.  The enumeration of the call sites was tool-assisted; the
classification and both changes were checked by hand against the
callers and against the teardown paths.

Instruction-level evidence (new): the two helpers do compile differently
on a weakly ordered CPU.  Built from this series' base (7.4.0-rc2) with
aarch64-linux-gnu-gcc 14.2 and arm64 defconfig, which enables
CONFIG_ARM64_USE_LSE_ATOMICS, net/core/sock_reuseport.o disassembles as:

  before 2/2 applied:
        fc4:  88a37c02        cas     w3, w2, [x0]     <!-- relaxed CAS -->
        fc8:  6b03003f        cmp     w1, w3
        fcc:  54000981        b.ne    ...

  with 2/2 applied:
        fd4:  88e17c43        casa    w1, w3, [x2]     <!-- acquire CAS -->
        fd8:  6b01001f        cmp     w0, w1

  i.e. CAS -> CASA.  For builds without LSE the same change turns the
  ldxr/stxr loop into ldaxr/stlxr, and refcount_set_release() turns the
  plain str into stlr.

On x86-64 both variants compile to the same "lock cmpxchg", so the
series is a no-op there, which is why the ordering violation is not
observable on the machine I tested on.

Reproduce:
  make ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- net/core/sock_reuseport.o
  aarch64-linux-gnu-objdump -d net/core/sock_reuseport.o \
        --disassemble=reuseport_migrate_sock

Whether it was triggered: no.  This is a theoretical ordering gap, not
an observed crash.  I have no syzbot report, no KCSAN splat and no
runtime reproducer for the reordering itself; demonstrating that would
need a weakly ordered machine (the instruction sequences above are the
strongest evidence I can produce here).  So the series does not claim to
fix an observed bug.

Reachability of the path (for completeness): with
net.ipv4.tcp_migrate_req=1 and four SO_REUSEPORT listeners on loopback,
a 20 second churn run (4 connector threads, 2 threads closing and
re-adding listeners) moved TCPMigrateReqSuccess from 16426 to 17103 and
TCPMigrateReqFailure from 1858 to 1875 on 6.18.58.  On 7.4.0-rc2 the
same load drove 6,625 successful migrations across three 80 second runs
(KASAN kernel, no reports).  A minimal two-listener case (64 established
but unaccepted connections, then close one listener) moves the counter
by 27-29.  This is only a reachability check, not a demonstration of the
reordering.



What was verified: the re-validation exists and is the guard described
above; the teardown order is __sk_nulls_del_node_init_rcu() followed by
WRITE_ONCE(msk->token, 0) in mptcp_token_destroy(); both touched objects
compile cleanly (net/mptcp/token.o, net/core/sock_reuseport.o) on x86-64
and arm64; the change is a no-op on strongly ordered CPUs.  The argument
is the same one used in 5b5d8f351dd9 for the inet lookups: a successful
reference acquisition has to order the validation that follows it.

Happy to drop the series or fold it into a larger ordering series if
reviewers would rather wait for a KCSAN/arm64 runtime demonstration.

Hang Nan

[-- Attachment #2: arm64-unpatched.txt --]
[-- Type: text/plain, Size: 671 bytes --]

     fac:	b9408261 	ldr	w1, [x19, #128]
     fb0:	91020260 	add	x0, x19, #0x80
     fb4:	340005a1 	cbz	w1, 1068 <reuseport_migrate_sock+0x148>
     fb8:	11000422 	add	w2, w1, #0x1
     fbc:	14000010 	b	ffc <reuseport_migrate_sock+0xdc>
     fc0:	2a0103e3 	mov	w3, w1
     fc4:	88a37c02 	cas	w3, w2, [x0]
     fc8:	6b03003f 	cmp	w1, w3
     fcc:	54000981 	b.ne	10fc <reuseport_migrate_sock+0x1dc>  // b.any
     fd0:	2a010042 	orr	w2, w2, w1
     fd4:	37f80502 	tbnz	w2, #31, 1074 <reuseport_migrate_sock+0x154>
     fd8:	f94023f9 	ldr	x25, [sp, #64]
     fdc:	94000000 	bl	0 <__rcu_read_unlock>
     fe0:	aa1303e0 	mov	x0, x19
     fe4:	a94153f3 	ldp	x19, x20, [sp, #16]

[-- Attachment #3: arm64-patched.txt --]
[-- Type: text/plain, Size: 688 bytes --]

     fac:	b9408260 	ldr	w0, [x19, #128]
     fb0:	91020262 	add	x2, x19, #0x80
     fb4:	12b00004 	mov	w4, #0x7fffffff            	// #2147483647
     fb8:	4b000081 	sub	w1, w4, w0
     fbc:	340005a0 	cbz	w0, 1070 <reuseport_migrate_sock+0x150>
     fc0:	7100003f 	cmp	w1, #0x0
     fc4:	5400056d 	b.le	1070 <reuseport_migrate_sock+0x150>
     fc8:	11000403 	add	w3, w0, #0x1
     fcc:	1400000e 	b	1004 <reuseport_migrate_sock+0xe4>
     fd0:	2a0003e1 	mov	w1, w0
     fd4:	88e17c43 	casa	w1, w3, [x2]
     fd8:	6b01001f 	cmp	w0, w1
     fdc:	540007e1 	b.ne	10d8 <reuseport_migrate_sock+0x1b8>  // b.any
     fe0:	f94023f9 	ldr	x25, [sp, #64]
     fe4:	94000000 	bl	0 <__rcu_read_unlock>

[-- Attachment #4: x86-unpatched.txt --]
[-- Type: text/plain, Size: 479 bytes --]

     865:	85 d2                	test   %edx,%edx
     867:	74 2a                	je     893 <reuseport_migrate_sock+0xc3>
     869:	8d 4a 01             	lea    0x1(%rdx),%ecx
     86c:	89 d0                	mov    %edx,%eax
     86e:	f0 0f b1 0f          	lock cmpxchg %ecx,(%rdi)
     872:	75 5f                	jne    8d3 <reuseport_migrate_sock+0x103>
     874:	09 ca                	or     %ecx,%edx
     876:	78 5f                	js     8d7 <reuseport_migrate_sock+0x107>

[-- Attachment #5: x86-patched.txt --]
[-- Type: text/plain, Size: 477 bytes --]

     870:	29 c1                	sub    %eax,%ecx
     872:	85 c9                	test   %ecx,%ecx
     874:	7e 24                	jle    89a <reuseport_migrate_sock+0xca>
     876:	8d 48 01             	lea    0x1(%rax),%ecx
     879:	f0 0f b1 0a          	lock cmpxchg %ecx,(%rdx)
     87d:	75 eb                	jne    86a <reuseport_migrate_sock+0x9a>
     87f:	e8 00 00 00 00       	call   884 <reuseport_migrate_sock+0xb4>
     884:	48 89 d8             	mov    %rbx,%rax