Skip to contentSkip to conversation

net/sched: cls_flow: pin file with get_file_rcu before reading f_cred

PATCH New Tristan Madani 2 participants 2 messages Nov 11

Subsystems (4)
Networking Network drivers Stable branch Traffic control (tc)
lore
Share & tools
lore

Tristan Madani

author
#0 lore raw

PATCH 1/1 net/sched: cls_flow: pin file with get_file_rcu before reading f_cred

From: Tristan Madani <[email protected]>

flow_get_skuid() and flow_get_skgid() read sk->sk_socket->file->f_cred without holding a reference to the file. Since the file slab uses SLAB_TYPESAFE_BY_RCU, a concurrent close() may free and recycle the file object, causing stale credential data to be read through f_cred.

Pin the file with get_file_rcu() before accessing its credentials and release with fput() afterwards.

Fixes 0ede61d8589cc ("file: convert to SLAB_TYPESAFE_BY_RCU")
Cc [email protected]
Signed-off-by Tristan Madani <[email protected]>
---
 net/sched/cls_flow.c | 21 +++++++++++++++++----
 1 file changed, 17 insertions(+), 4 deletions(-)
net/sched/cls_flow.c +17 -4
index a9ac3acf6eda0..233ca13299930 100644
@@ -17,6 +17,7 @@
 #include <linux/ipv6.h>
 #include <linux/if_vlan.h>
 #include <linux/slab.h>
+#include <linux/fs.h>
 #include <linux/module.h>
 #include <net/inet_sock.h>
 

@@ -208,10 +209,16 @@ static u32 flow_get_rtclassid(const struct sk_buff *skb)
 static u32 flow_get_skuid(const struct sk_buff *skb)
 {
 	struct sock *sk = skb_to_full_sk(skb);
+	struct file *filp;
 
-	if (sk && sk->sk_socket && sk->sk_socket->file) {
-		kuid_t skuid = sk->sk_socket->file->f_cred->fsuid;
+	if (!sk || !sk->sk_socket)
+		return 0;
+
+	filp = get_file_rcu(&sk->sk_socket->file);
+	if (filp) {
+		kuid_t skuid = filp->f_cred->fsuid;
 
+		fput(filp);
 		return from_kuid(&init_user_ns, skuid);
 	}
 	return 0;

@@ -220,10 +227,16 @@ static u32 flow_get_skuid(const struct sk_buff *skb)
 static u32 flow_get_skgid(const struct sk_buff *skb)
 {
 	struct sock *sk = skb_to_full_sk(skb);
+	struct file *filp;
+
+	if (!sk || !sk->sk_socket)
+		return 0;
 
-	if (sk && sk->sk_socket && sk->sk_socket->file) {
-		kgid_t skgid = sk->sk_socket->file->f_cred->fsgid;
+	filp = get_file_rcu(&sk->sk_socket->file);
+	if (filp) {
+		kgid_t skgid = filp->f_cred->fsgid;
 
+		fput(filp);
 		return from_kgid(&init_user_ns, skgid);
 	}
 	return 0;
Signature · 1 line
2.47.3
AI review
#1 lore raw
This is an automated message. This series looks like a fix, but its commit messages seem to be missing some information:

Hi!

This is an automated message. This series looks like a fix, but its commit messages seem to be missing some information:

- How the issue was discovered, e.g. hit in production, hit during development, syzbot report, manual code inspection, LLM or static analysis tool scan.

- Whether the issue was actually triggered, or is only theoretical (e.g. found by LLM and not proven at runtime). If it was triggered please include the symptoms, like the stack trace or kernel messages.

Please do not repost the series just to address the above. Instead, reply to this email with the missing information, so that reviewers can take it into account. If the series needs another revision for other reasons, please include the information in the commit messages then.

The evaluation is done by an LLM so it may be wrong, if you think that is the case please reply and explain.