[RFC PATCH 04/41] net: sock: read sk_uid through a seqcount

[Jan Koester]

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 <jan.koester@xxxxxxx>
---
 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