sk->sk_flags must only be changed while holding the socket lock, because sock_set_flag() and sock_reset_flag() use non atomic operations (__set_bit() and __clear_bit()). sock_gettstamp() is one of the last places where a bit of sk->sk_flags is changed from a syscall without owning the socket lock, through sock_enable_timestamp(sk, SOCK_TIMESTAMP). sk_set_memalloc() and sk_clear_memalloc() also change sk->sk_flags without the socket lock, but their callers (nbd, iscsi_tcp, nvme-tcp, sunrpc, wireguard) need a careful audit, this will be addressed in a separate patch. Jungwoo Lee and Wongi Lee reported an UDP socket use-after-free caused by this bug: a SIOCGSTAMPNS_NEW ioctl racing with bind() can cancel the SOCK_RCU_FREE bit that udp_lib_get_port() just set, because both threads perform a read-modify-write on the same word. CPU 0 (bind) CPU 1 (SIOCGSTAMPNS_NEW) -------------------------------- ---------------------------- read sk_flags = F read sk_flags = F compute F | BIT(SOCK_RCU_FREE) compute F | BIT(SOCK_TIMESTAMP) store F | BIT(SOCK_RCU_FREE) sk_add_node_rcu(sk, ...) store F | BIT(SOCK_TIMESTAMP) After the lost update, SOCK_RCU_FREE is clear while the socket is visible to lockless UDP receive lookups. sk_destruct() then frees the socket immediately instead of waiting for a RCU grace period, while the receive path still holds a reference-less pointer to it: BUG: KASAN: slab-use-after-free in ipv4_pktinfo_prepare+0x30/0x410 Read of size 8 at addr ffff888008806610 by task exploit/207 CPU: 0 UID: 1000 PID: 207 Comm: exploit Not tainted 6.12.95+ #1 ipv4_pktinfo_prepare+0x30/0x410 udp_queue_rcv_one_skb+0x51c/0x1180 udp_unicast_rcv_skb+0x109/0x350 ip_protocol_deliver_rcu+0x14b/0x310 ip_local_deliver_finish+0x29d/0x390 ip_local_deliver+0x24d/0x2a0 Only grab the socket lock when SOCK_TIMESTAMP has to be set, to keep the common case lockless. Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") Reported-by: Jungwoo Lee Reported-by: Wongi Lee Signed-off-by: Eric Dumazet --- net/core/sock.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/net/core/sock.c b/net/core/sock.c index fa60b7494c58691d3f5d34a62d2176c87086424a..d5e302e21e85b0637a3232f5a3e9dace9978e8ee 100644 --- a/net/core/sock.c +++ b/net/core/sock.c @@ -3911,7 +3911,14 @@ int sock_gettstamp(struct socket *sock, void __user *userstamp, struct sock *sk = sock->sk; struct timespec64 ts; - sock_enable_timestamp(sk, SOCK_TIMESTAMP); + /* sk->sk_flags must only be changed under the socket lock, + * because sock_set_flag() uses non atomic operations. + */ + if (!sock_flag(sk, SOCK_TIMESTAMP)) { + lock_sock(sk); + sock_enable_timestamp(sk, SOCK_TIMESTAMP); + release_sock(sk); + } ts = ktime_to_timespec64(sock_read_timestamp(sk)); if (ts.tv_sec == -1) return -ENOENT; -- 2.55.0.1032.g73a4cd73de-goog