sk_uid is read without a lock, with READ_ONCE(), by the routing lookups and the reuseport checks, while sockfs_setattr() may change it. That only works for as long as a kuid_t is a single word, which it won't be once ids can be UUIDs and GUIDs.
Add sk_uid_seq next to it: writers bump it around the store and hold sk_callback_lock (sock_graft(), sockfs_setattr()) or own the socket alone (sock_init_data_uid()); sk_uid() retries until it sees a whole id. sk_clone() reads the parent's id the same way rather than relying on the copy made by sock_copy(). Assisted-by: Claude:claude-opus-5-5 Signed-off-by: Jan Koester <[email protected]> --- include/net/sock.h | 28 +++++++++++++++++++++++++--- net/core/sock.c | 6 +++++- net/socket.c | 5 +++-- 3 files changed, 33 insertions(+), 6 deletions(-) diff --git a/include/net/sock.h b/include/net/sock.h index 60ea55dc18..0986d94665 100644 --- a/include/net/sock.h +++ b/include/net/sock.h @@ -294,6 +294,8 @@ struct sk_filter; * @sk_ack_backlog: current listen backlog * @sk_max_ack_backlog: listen backlog set in listen() * @sk_uid: user id of owner + * @sk_uid_seq: makes lockless reads of @sk_uid see a whole id; writers + * hold @sk_callback_lock * @sk_ino: inode number (zero if orphaned) * @sk_prefer_busy_poll: prefer busypolling over softirq processing * @sk_busy_poll_budget: napi processing budget when busypolling @@ -509,6 +511,7 @@ struct sock { u32 sk_priority; u32 sk_mark; kuid_t sk_uid; + seqcount_t sk_uid_seq; u16 sk_protocol; u16 sk_type; struct dst_entry __rcu *sk_dst_cache; @@ -2109,11 +2112,23 @@ static inline int sk_rx_queue_get(const struct sock *sk) return -1; } +/* + * sk_uid is too big for WRITE_ONCE(): writers hold sk_callback_lock (or own + * the socket alone, before it is published) and bump sk_uid_seq around the + * store, so sk_uid() can read it without a lock. + */ +static inline void __sk_set_uid(struct sock *sk, kuid_t uid) +{ + write_seqcount_begin(&sk->sk_uid_seq); + sk->sk_uid = uid; + write_seqcount_end(&sk->sk_uid_seq); +} + static inline void sk_set_socket(struct sock *sk, struct socket *sock) { WRITE_ONCE(sk->sk_socket, sock); if (sock) { - WRITE_ONCE(sk->sk_uid, SOCK_INODE(sock)->i_uid); + __sk_set_uid(sk, SOCK_INODE(sock)->i_uid); WRITE_ONCE(sk->sk_ino, SOCK_INODE(sock)->i_ino); } else { /* Note: sk_uid is unchanged. */ @@ -2161,8 +2176,15 @@ static inline u64 sock_i_ino(const struct sock *sk) static inline kuid_t sk_uid(const struct sock *sk) { - /* Paired with WRITE_ONCE() in sockfs_setattr() */ - return READ_ONCE(sk->sk_uid); + unsigned int seq; + kuid_t uid; + + /* Paired with __sk_set_uid() */ + do { + seq = read_seqcount_begin(&sk->sk_uid_seq); + uid = sk->sk_uid; + } while (read_seqcount_retry(&sk->sk_uid_seq, seq)); + return uid; } static inline kuid_t sock_net_uid(const struct net *net, const struct sock *sk) diff --git a/net/core/sock.c b/net/core/sock.c index e8551df833..300bc75ce1 100644 --- a/net/core/sock.c +++ b/net/core/sock.c @@ -2449,6 +2449,7 @@ static void sk_init_common(struct sock *sk) skb_queue_head_init(&sk->sk_error_queue); rwlock_init(&sk->sk_callback_lock); + seqcount_init(&sk->sk_uid_seq); lockdep_set_class_and_name(&sk->sk_receive_queue.lock, af_rlock_keys + sk->sk_family, af_family_rlock_key_strings[sk->sk_family]); @@ -2529,6 +2530,8 @@ struct sock *sk_clone(const struct sock *sk, const gfp_t priority, atomic_set(&newsk->sk_omem_alloc, 0); sk_init_common(newsk); + /* sock_copy() may have caught a half written sk_uid */ + newsk->sk_uid = sk_uid(sk); newsk->sk_dst_cache = NULL; newsk->sk_dst_pending_confirm = 0; @@ -3773,7 +3776,7 @@ void sock_init_data_uid(struct socket *sock, struct sock *sk, kuid_t uid) } else { RCU_INIT_POINTER(sk->sk_wq, NULL); } - sk->sk_uid = uid; + __sk_set_uid(sk, uid); sk->sk_state_change = sock_def_wakeup; sk->sk_data_ready = sock_def_readable; @@ -4614,6 +4617,7 @@ static int __init sock_struct_check(void) CACHELINE_ASSERT_GROUP_MEMBER(struct sock, sock_read_tx, sk_priority); CACHELINE_ASSERT_GROUP_MEMBER(struct sock, sock_read_tx, sk_mark); CACHELINE_ASSERT_GROUP_MEMBER(struct sock, sock_read_tx, sk_uid); + CACHELINE_ASSERT_GROUP_MEMBER(struct sock, sock_read_tx, sk_uid_seq); CACHELINE_ASSERT_GROUP_MEMBER(struct sock, sock_read_tx, sk_protocol); CACHELINE_ASSERT_GROUP_MEMBER(struct sock, sock_read_tx, sk_dst_cache); CACHELINE_ASSERT_GROUP_MEMBER(struct sock, sock_read_tx, sk_route_caps); diff --git a/net/socket.c b/net/socket.c index c05d86e63a..09658f3958 100644 --- a/net/socket.c +++ b/net/socket.c @@ -679,8 +679,9 @@ static int sockfs_setattr(struct mnt_idmap *idmap, struct socket *sock = SOCKET_I(d_inode(dentry)); if (sock->sk) { - /* Paired with READ_ONCE() in sk_uid() */ - WRITE_ONCE(sock->sk->sk_uid, iattr->ia_uid); + write_lock_bh(&sock->sk->sk_callback_lock); + __sk_set_uid(sock->sk, iattr->ia_uid); + write_unlock_bh(&sock->sk->sk_callback_lock); } else { err = -ENOENT; } -- 2.56.0
