[PATCH net 0/2] connector/cn_proc: fix listener count and socket state cleanup

[PATCH net 0/2] connector/cn_proc: fix listener count and socket state cleanup

[Cen Zhang (Microsoft)]

Two independent bugs in the process events connector, both reachable
by an unprivileged user:

1. proc_event_num_listeners can go negative because cn_proc_mcast_ctl()
   does not validate mcast_op before storing it as socket state. All
   process events are then suppressed for every subscriber.

2. sk_user_data leaks and the listener count stays too high because
   cn_release() frees the state only for multicast group members and
   never drops the count.

Both were found and fixed with the assistance of an LLM-based code
analysis tool and verified by the author.

Cen Zhang (Microsoft) (2):
  connector/cn_proc: reject invalid mcast_op before touching socket
    state
  connector/cn_proc: release socket state and listener count on close

 drivers/connector/cn_proc.c   | 23 ++++++++++++++++++++---
 drivers/connector/connector.c |  7 +++----
 include/linux/cn_proc.h       |  6 ++++++
 3 files changed, 29 insertions(+), 7 deletions(-)


base-commit: 94cea342e8915812b45f93c56e134041c7fdb4d7
-- 
2.55.0

[PATCH net 1/2] connector/cn_proc: reject invalid mcast_op before touching socket state

[Cen Zhang (Microsoft)]

cn_proc_mcast_ctl() does not validate mcast_op before storing it in the
per-socket proc_input, so an invalid value is kept as the socket's
state. The IGNORE case only tests prev_mc_op != PROC_CN_MCAST_IGNORE, so
a following PROC_CN_MCAST_IGNORE decrements proc_event_num_listeners for
a socket that never did LISTEN.

Since commit bfdfdc2f3b70
("connector/cn_proc: Allow non-root users access") any user in the initial
namespaces can send these messages. Alternating an invalid operation and
IGNORE drives the count negative; every proc_*_connector() then returns
early and no process events are delivered to any subscriber, including
root-owned monitors.

Fix by rejecting an operation other than LISTEN or IGNORE before any
per-socket state is created or modified. The default case of the switch
becomes unreachable and is removed.

Fixes: 2aa1f7a1f47c ("connector/cn_proc: Add filtering to fix some bugs")
Cc: stable@vger.kernel.org
Cc: AutonomousCodeSecurity@microsoft.com
Signed-off-by: Cen Zhang (Microsoft) <cenzhang@linux.microsoft.com>
---
The bug was found and this fix was developed with the assistance of an
LLM-based code analysis tool. The analysis, the patch and its testing
were reviewed and verified by the author.

 drivers/connector/cn_proc.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/connector/cn_proc.c b/drivers/connector/cn_proc.c
index 0056ab81fbc3..4f613c0648ae 100644
--- a/drivers/connector/cn_proc.c
+++ b/drivers/connector/cn_proc.c
@@ -421,6 +421,12 @@ static void cn_proc_mcast_ctl(struct cn_msg *msg,
 		return;
 	}
 
+	if (mc_op != PROC_CN_MCAST_LISTEN &&
+	    mc_op != PROC_CN_MCAST_IGNORE) {
+		err = EINVAL;
+		goto out;
+	}
+
 	ev_type = valid_event((enum proc_cn_event)ev_type);
 
 	if (ev_type == PROC_EVENT_NONE)
@@ -455,9 +461,6 @@ static void cn_proc_mcast_ctl(struct cn_msg *msg,
 		((struct proc_input *)(sk->sk_user_data))->event_type =
 			PROC_EVENT_NONE;
 		break;
-	default:
-		err = EINVAL;
-		break;
 	}
 
 out:
-- 
2.55.0

[PATCH net 2/2] connector/cn_proc: release socket state and listener count on close

[Cen Zhang (Microsoft)]

cn_release() is the netlink release callback for NETLINK_CONNECTOR
sockets. It frees the per-socket proc_input only if the socket is a
member of the CN_IDX_PROC multicast group, and it never touches
proc_event_num_listeners. Both are wrong: cn_proc_mcast_ctl() allocates
proc_input for any socket that sends a control message, and a socket
that is still in LISTEN state when it closes has incremented the count
and never decremented it.

This has two consequences. A socket that sends a control message
without joining the group leaks its proc_input on close; an
unprivileged user looping over socket(), one control message and
close() leaks 8 bytes of kmalloc-8 per iteration that no process exit
gives back, until the OOM killer fires:

  Out of memory: Killed process 93 (trigger) total-vm:1016kB, ... UID:1000

And a listener that exits without sending IGNORE leaves the count too
high, so the kernel keeps building process events that nobody receives.

Fix by moving the cleanup into cn_proc.c and running it for every
socket on close: free proc_input if present, and decrement the count if
the socket was still in LISTEN state.

Fixes: 2aa1f7a1f47c ("connector/cn_proc: Add filtering to fix some bugs")
Cc: stable@vger.kernel.org
Cc: AutonomousCodeSecurity@microsoft.com
Signed-off-by: Cen Zhang (Microsoft) <cenzhang@linux.microsoft.com>
---
The bug was found and this fix was developed with the assistance of an
LLM-based code analysis tool. The analysis, the patch and its testing
were reviewed and verified by the author.

 drivers/connector/cn_proc.c   | 14 ++++++++++++++
 drivers/connector/connector.c |  7 +++----
 include/linux/cn_proc.h       |  6 ++++++
 3 files changed, 23 insertions(+), 4 deletions(-)

diff --git a/drivers/connector/cn_proc.c b/drivers/connector/cn_proc.c
index 4f613c0648ae..a4b62998095e 100644
--- a/drivers/connector/cn_proc.c
+++ b/drivers/connector/cn_proc.c
@@ -387,6 +387,20 @@ static void cn_proc_ack(int err, int rcvd_seq, int rcvd_ack)
 	send_msg(msg);
 }
 
+/* Called when a NETLINK_CONNECTOR socket is released. */
+void cn_proc_release(struct sock *sk)
+{
+	struct proc_input *input = sk->sk_user_data;
+
+	if (!input)
+		return;
+
+	if (input->mcast_op == PROC_CN_MCAST_LISTEN)
+		atomic_dec(&proc_event_num_listeners);
+	kfree(input);
+	sk->sk_user_data = NULL;
+}
+
 /**
  * cn_proc_mcast_ctl
  * @msg: message sent from userspace via the connector
diff --git a/drivers/connector/connector.c b/drivers/connector/connector.c
index 4028e8eeba82..f31ee8962f7c 100644
--- a/drivers/connector/connector.c
+++ b/drivers/connector/connector.c
@@ -14,6 +14,7 @@
 #include <net/netlink.h>
 #include <linux/moduleparam.h>
 #include <linux/connector.h>
+#include <linux/cn_proc.h>
 #include <linux/slab.h>
 #include <linux/mutex.h>
 #include <linux/proc_fs.h>
@@ -182,12 +183,10 @@ static int cn_bind(struct net *net, int group)
 	return -EPERM;
 }
 
+/* sk_user_data of a connector socket belongs to cn_proc. */
 static void cn_release(struct sock *sk, unsigned long *groups)
 {
-	if (groups && test_bit(CN_IDX_PROC - 1, groups)) {
-		kfree(sk->sk_user_data);
-		sk->sk_user_data = NULL;
-	}
+	cn_proc_release(sk);
 }
 
 /*
diff --git a/include/linux/cn_proc.h b/include/linux/cn_proc.h
index 1d5b02a96c46..8de921d731ce 100644
--- a/include/linux/cn_proc.h
+++ b/include/linux/cn_proc.h
@@ -19,6 +19,8 @@
 
 #include <uapi/linux/cn_proc.h>
 
+struct sock;
+
 #ifdef CONFIG_PROC_EVENTS
 void proc_fork_connector(struct task_struct *task);
 void proc_exec_connector(struct task_struct *task);
@@ -28,6 +30,7 @@ void proc_ptrace_connector(struct task_struct *task, int which_id);
 void proc_comm_connector(struct task_struct *task);
 void proc_coredump_connector(struct task_struct *task);
 void proc_exit_connector(struct task_struct *task);
+void cn_proc_release(struct sock *sk);
 #else
 static inline void proc_fork_connector(struct task_struct *task)
 {}
@@ -54,5 +57,8 @@ static inline void proc_coredump_connector(struct task_struct *task)
 
 static inline void proc_exit_connector(struct task_struct *task)
 {}
+
+static inline void cn_proc_release(struct sock *sk)
+{}
 #endif	/* CONFIG_PROC_EVENTS */
 #endif	/* CN_PROC_H */
-- 
2.55.0