[v1,2/5] connector/cn_proc: Add filtering to fix some bugs

Message ID 20230310221547.3656194-3-anjali.k.kulkarni@oracle.com
State New
Headers
Series Process connector bug fixes & enhancements |

Commit Message

Anjali Kulkarni March 10, 2023, 10:15 p.m. UTC
  One bug is if there are more than one listeners for the proc connector
messages, and one of them deregisters for listening using
PROC_CN_MCAST_IGNORE, they will still get all proc connector messages,
as long as there is another listener.

Another issue is if one client calls PROC_CN_MCAST_LISTEN, and another
one calls PROC_CN_MCAST_IGNORE, then both will end up not getting any
messages.

This patch adds filtering and drops packet if client has sent
PROC_CN_MCAST_IGNORE. This data is stored in the client socket's
sk_user_data. In addition, we only increment or decrement
proc_event_num_listeners once per client. This fixes the above issues.

Signed-off-by: Anjali Kulkarni <anjali.k.kulkarni@oracle.com>
---
 drivers/connector/cn_proc.c   | 53 ++++++++++++++++++++++++++++-------
 drivers/connector/connector.c | 12 +++++---
 drivers/w1/w1_netlink.c       |  6 ++--
 include/linux/connector.h     |  6 +++-
 include/uapi/linux/cn_proc.h  | 43 ++++++++++++++++------------
 net/netlink/af_netlink.c      | 10 +++++--
 6 files changed, 93 insertions(+), 37 deletions(-)
  

Comments

Jakub Kicinski March 14, 2023, 12:24 a.m. UTC | #1
On Fri, 10 Mar 2023 14:15:44 -0800 Anjali Kulkarni wrote:
> diff --git a/include/linux/connector.h b/include/linux/connector.h
> index 487350bb19c3..1336a5e7dd2f 100644
> --- a/include/linux/connector.h
> +++ b/include/linux/connector.h
> @@ -96,7 +96,11 @@ void cn_del_callback(const struct cb_id *id);
>   *
>   * If there are no listeners for given group %-ESRCH can be returned.
>   */
> -int cn_netlink_send_mult(struct cn_msg *msg, u16 len, u32 portid, u32 group, gfp_t gfp_mask);
> +int cn_netlink_send_mult(struct cn_msg *msg, u16 len, u32 portid,
> +			 u32 group, gfp_t gfp_mask,
> +			 int (*filter)(struct sock *dsk, struct sk_buff *skb,
> +				       void *data),
> +			 void *filter_data);

kdoc needs to be extended

> diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
> index 003c7e6ec9be..b311375b8c4c 100644
> --- a/net/netlink/af_netlink.c
> +++ b/net/netlink/af_netlink.c
> @@ -63,6 +63,7 @@
>  #include <linux/net_namespace.h>
>  #include <linux/nospec.h>
>  #include <linux/btf_ids.h>
> +#include <linux/connector.h>
>  
>  #include <net/net_namespace.h>
>  #include <net/netns/generic.h>
> @@ -767,9 +768,14 @@ static int netlink_release(struct socket *sock)
>  	/* must not acquire netlink_table_lock in any way again before unbind
>  	 * and notifying genetlink is done as otherwise it might deadlock
>  	 */
> -	if (nlk->netlink_unbind) {
> +	if (nlk->netlink_unbind && nlk->groups) {
>  		int i;
> -
> +		if (sk->sk_protocol == NETLINK_CONNECTOR) {
> +			if (test_bit(CN_IDX_PROC - 1, nlk->groups)) {
> +				kfree(sk->sk_user_data);
> +				sk->sk_user_data = NULL;
> +			}
> +		}
>  		for (i = 0; i < nlk->ngroups; i++)
>  			if (test_bit(i, nlk->groups))
>  				nlk->netlink_unbind(sock_net(sk), i + 1);

This is clearly a layering violation, right?
Please don't add "if (family_x)" to the core netlink code.
  
Anjali Kulkarni March 14, 2023, 2:32 a.m. UTC | #2
> -int cn_netlink_send_mult(struct cn_msg *msg, u16 len, u32 portid, u32 group, gfp_t gfp_mask);
> +int cn_netlink_send_mult(struct cn_msg *msg, u16 len, u32 portid,
> +                      u32 group, gfp_t gfp_mask,
> +                      int (*filter)(struct sock *dsk, struct sk_buff *skb,
> +                                    void *data),
> +                      void *filter_data);

kdoc needs to be extended
ANJALI> Thanks, will update in next revision.

> -
> +             if (sk->sk_protocol == NETLINK_CONNECTOR) {
> +                     if (test_bit(CN_IDX_PROC - 1, nlk->groups)) {
> +                             kfree(sk->sk_user_data);
> +                             sk->sk_user_data = NULL;
> +                     }
> +             }
>                for (i = 0; i < nlk->ngroups; i++)
>                        if (test_bit(i, nlk->groups))
>                                nlk->netlink_unbind(sock_net(sk), i + 1);

This is clearly a layering violation, right?
Please don't add "if (family_x)" to the core netlink code.
ANJALI> Yes, it is, but there does not seem a very clean way to do it otherwise and I saw a check for protocol NETLINK_GENERIC just below it, so used it for connector as well. There is no release or free callback in the netlink_sock. Is it ok to add it? There was another bug (for which I have not yet sent a patch) in which, we need to decrement proc_event_num_listeners, when client exits without calling IGNORE, else that count again gets out of status of actual no of listeners. 
The other option is to add a flag in netlink_sock, something like NETLINK_F_SK_USER_DATA_FREE, which will free the sk_user_data, if this flag is set. But it does not solve the above scenario.
.
  
Christian Brauner March 14, 2023, 8:38 a.m. UTC | #3
On Mon, Mar 13, 2023 at 05:24:41PM -0700, Jakub Kicinski wrote:
> On Fri, 10 Mar 2023 14:15:44 -0800 Anjali Kulkarni wrote:
> > diff --git a/include/linux/connector.h b/include/linux/connector.h
> > index 487350bb19c3..1336a5e7dd2f 100644
> > --- a/include/linux/connector.h
> > +++ b/include/linux/connector.h
> > @@ -96,7 +96,11 @@ void cn_del_callback(const struct cb_id *id);
> >   *
> >   * If there are no listeners for given group %-ESRCH can be returned.
> >   */
> > -int cn_netlink_send_mult(struct cn_msg *msg, u16 len, u32 portid, u32 group, gfp_t gfp_mask);
> > +int cn_netlink_send_mult(struct cn_msg *msg, u16 len, u32 portid,
> > +			 u32 group, gfp_t gfp_mask,
> > +			 int (*filter)(struct sock *dsk, struct sk_buff *skb,
> > +				       void *data),
> > +			 void *filter_data);
> 
> kdoc needs to be extended

just a thought from my side. I think giving access to unprivileged users
will require a little thought as that's potentially sensitive.

If possible I would think that the patches that don't lead to a
behavioral change should go in completely independently and then we can
discuss the non-root access change.
  
Jakub Kicinski March 15, 2023, 4:59 a.m. UTC | #4
On Tue, 14 Mar 2023 02:32:13 +0000 Anjali Kulkarni wrote:
> This is clearly a layering violation, right?
> Please don't add "if (family_x)" to the core netlink code.
>
> ANJALI> Yes, it is, but there does not seem a very clean way to do it
> ANJALI> otherwise and I saw a check for protocol NETLINK_GENERIC just
> ANJALI> below it, so used it for connector as well. There is no
> ANJALI> release or free callback in the netlink_sock. Is it ok to add
> ANJALI> it? There was another bug (for which I have not yet sent a
> ANJALI> patch) in which, we need to decrement
> ANJALI> proc_event_num_listeners, when client exits without calling
> ANJALI> IGNORE, else that count again gets out of status of actual no
> ANJALI> of listeners.   
> The other option is to add a flag in netlink_sock, something like
> NETLINK_F_SK_USER_DATA_FREE, which will free the sk_user_data, if
> this flag is set. But it does not solve the above scenario.

Please fix your email setup, it's really hard to read your replies.

There is an unbind callback, and a notifier. Can neither of those 
be made to work? ->sk_user_data is not a great choice of a field,
either, does any other netlink family use it this way?
Adding a new field for family use to struct netlink_sock may be better.
  
Anjali Kulkarni March 15, 2023, 7:08 p.m. UTC | #5
> On Mar 14, 2023, at 9:59 PM, Jakub Kicinski <kuba@kernel.org> wrote:
> 
> On Tue, 14 Mar 2023 02:32:13 +0000 Anjali Kulkarni wrote:
>> This is clearly a layering violation, right?
>> Please don't add "if (family_x)" to the core netlink code.
>> 
>> ANJALI> Yes, it is, but there does not seem a very clean way to do it
>> ANJALI> otherwise and I saw a check for protocol NETLINK_GENERIC just
>> ANJALI> below it, so used it for connector as well. There is no
>> ANJALI> release or free callback in the netlink_sock. Is it ok to add
>> ANJALI> it? There was another bug (for which I have not yet sent a
>> ANJALI> patch) in which, we need to decrement
>> ANJALI> proc_event_num_listeners, when client exits without calling
>> ANJALI> IGNORE, else that count again gets out of status of actual no
>> ANJALI> of listeners.   
>> The other option is to add a flag in netlink_sock, something like
>> NETLINK_F_SK_USER_DATA_FREE, which will free the sk_user_data, if
>> this flag is set. But it does not solve the above scenario.
> 
> Please fix your email setup, it's really hard to read your replies.
> 

I have changed my email client, let me know if this does not fix the issue you see.

> There is an unbind callback, and a notifier. Can neither of those 
> be made to work? ->sk_user_data is not a great choice of a field,
> either, does any other netlink family use it this way?
> Adding a new field for family use to struct netlink_sock may be better.

The unbind call will not work because it is for the case of adding and deleting group memberships and hence called from netlink_setsockopt() when  NETLINK_DROP_MEMBERSHIP option is given. We would not be able to distinguish between the drop membership & release cases.
The notifier call seems to be for blocked clients? Am not sure if the we need to block/wait on this call to be notified to free/release. Also, the API does not pass in struct sock to free what we want, so we will need to change that everywhere it is currently used.
As for using sk_user_data - this field seems to be used by different applications in different ways depending on how they need to use data. If we use a new field in netlink_sock, we would need to add a new API function to allocate this member, similar to release, because it seems you cannot access netlink_sock outside of af_netlink, or at least I do not see any current access to it, and functions like nlk_sk are static. Also, if we add an allocation function, we won’t know the first time the client sends it’s data (we need to know “initial” in the patches), so we will need to add a new field in the socket to indicate first access or add a lot more infrastructure in cn_proc to store each client’s information.

Anjali
  
Anjali Kulkarni March 15, 2023, 7:13 p.m. UTC | #6
access netlink_sock outside of af_netlink, or at least I do not see any current access to it, and functions like nlk_sk are static. Also, if we add an allocation function, we won’t know the first time the client sends it’s data (we need to know “initial” in the patches), so we will need to add a new field in the socket to indicate first access or add a lot more infrastructure in cn_proc to store each client’s information.

ANJALI> My mistake above - I guess we could store it as a new sub-field in the new netlink_sock structure.
  
Jakub Kicinski March 15, 2023, 7:50 p.m. UTC | #7
On Wed, 15 Mar 2023 19:08:34 +0000 Anjali Kulkarni wrote:
> > On Mar 14, 2023, at 9:59 PM, Jakub Kicinski <kuba@kernel.org> wrote:
> > 
> > On Tue, 14 Mar 2023 02:32:13 +0000 Anjali Kulkarni wrote:  
> >> This is clearly a layering violation, right?
> >> Please don't add "if (family_x)" to the core netlink code.
> >> The other option is to add a flag in netlink_sock, something like
> >> NETLINK_F_SK_USER_DATA_FREE, which will free the sk_user_data, if
> >> this flag is set. But it does not solve the above scenario.  
> > 
> > Please fix your email setup, it's really hard to read your replies.
> >   
> 
> I have changed my email client, let me know if this does not fix the issue you see.

Quite a bit better, thanks!

> > There is an unbind callback, and a notifier. Can neither of those 
> > be made to work? ->sk_user_data is not a great choice of a field,
> > either, does any other netlink family use it this way?
> > Adding a new field for family use to struct netlink_sock may be better.  
> 
> The unbind call will not work because it is for the case of adding and deleting group memberships and hence called from netlink_setsockopt() when  NETLINK_DROP_MEMBERSHIP option is given. We would not be able to distinguish between the drop membership & release cases.
> The notifier call seems to be for blocked clients? Am not sure if the we need to block/wait on this call to be notified to free/release. Also, the API does not pass in struct sock to free what we want, so we will need to change that everywhere it is currently used.

I think that adding the new release callback is acceptable.
I haven't seen your v2 before replying.

> As for using sk_user_data - this field seems to be used by different applications in different ways depending on how they need to use data. If we use a new field in netlink_sock, we would need to add a new API function to allocate this member, similar to release, because it seems you cannot access netlink_sock outside of af_netlink, or at least I do not see any current access to it, and functions like nlk_sk are static. Also, if we add an allocation function, we won’t know the first time the client sends it’s data (we need to know “initial” in the patches), so we will need to add a new field in the socket to indicate first access or add a lot more infrastructure in cn_proc to store each client’s information.

Alright, I guess we can risk the sk_user_data, and see if anything
explodes. Normally higher protocol wraps the socket structure in its
own structure and the sk_user_data pointer is for an in-kernel user
of the socket (i.e. in kernel client). But "classic netlink" is all
legacy code (new subsystem use the generic netlink overlay) so trying
to decode now why things are the way they are and fixing the structure
would be a major effort :(
  
Anjali Kulkarni March 15, 2023, 8:12 p.m. UTC | #8
>> 
>> The unbind call will not work because it is for the case of adding and deleting group memberships and hence called from netlink_setsockopt() when  NETLINK_DROP_MEMBERSHIP option is given. We would not be able to distinguish between the drop membership & release cases.
>> The notifier call seems to be for blocked clients? Am not sure if the we need to block/wait on this call to be notified to free/release. Also, the API does not pass in struct sock to free what we want, so we will need to change that everywhere it is currently used.
> 
> I think that adding the new release callback is acceptable.
> I haven't seen your v2 before replying.

Thanks, so I will wait for your comments on v2 before sending updated patch.

> 
>> As for using sk_user_data - this field seems to be used by different applications in different ways depending on how they need to use data. If we use a new field in netlink_sock, we would need to add a new API function to allocate this member, similar to release, because it seems you cannot access netlink_sock outside of af_netlink, or at least I do not see any current access to it, and functions like nlk_sk are static. Also, if we add an allocation function, we won’t know the first time the client sends it’s data (we need to know “initial” in the patches), so we will need to add a new field in the socket to indicate first access or add a lot more infrastructure in cn_proc to store each client’s information.
> 
> Alright, I guess we can risk the sk_user_data, and see if anything
> explodes. Normally higher protocol wraps the socket structure in its
> own structure and the sk_user_data pointer is for an in-kernel user
> of the socket (i.e. in kernel client). But "classic netlink" is all
> legacy code (new subsystem use the generic netlink overlay) so trying
> to decode now why things are the way they are and fixing the structure
> would be a major effort :(

Ok, thanks!
  
Anjali Kulkarni April 1, 2023, 6:32 p.m. UTC | #9
> On Mar 14, 2023, at 1:38 AM, Christian Brauner <brauner@kernel.org> wrote:
> 
> On Mon, Mar 13, 2023 at 05:24:41PM -0700, Jakub Kicinski wrote:
>> On Fri, 10 Mar 2023 14:15:44 -0800 Anjali Kulkarni wrote:
>>> diff --git a/include/linux/connector.h b/include/linux/connector.h
>>> index 487350bb19c3..1336a5e7dd2f 100644
>>> --- a/include/linux/connector.h
>>> +++ b/include/linux/connector.h
>>> @@ -96,7 +96,11 @@ void cn_del_callback(const struct cb_id *id);
>>> *
>>> * If there are no listeners for given group %-ESRCH can be returned.
>>> */
>>> -int cn_netlink_send_mult(struct cn_msg *msg, u16 len, u32 portid, u32 group, gfp_t gfp_mask);
>>> +int cn_netlink_send_mult(struct cn_msg *msg, u16 len, u32 portid,
>>> +			 u32 group, gfp_t gfp_mask,
>>> +			 int (*filter)(struct sock *dsk, struct sk_buff *skb,
>>> +				 void *data),
>>> +			 void *filter_data);
>> 
>> kdoc needs to be extended
> 
> just a thought from my side. I think giving access to unprivileged users
> will require a little thought as that's potentially sensitive.
> 
> If possible I would think that the patches that don't lead to a
> behavioral change should go in completely independently and then we can
> discuss the non-root access change.

Hi Christian,

Could you take a look at v4 and let me know your thoughts, so we can start a discussion on that thread? Do we need more filtering based on user ID /other parameters for exit status? Can we allow just non-zero notification (not the exact exit status but just whether it was a 0 or a non-zero exit) be available to non-root users? 

Do other folks have any more comments/suggestions?

Thanks
Anjali
  

Patch

diff --git a/drivers/connector/cn_proc.c b/drivers/connector/cn_proc.c
index ccac1c453080..84f38d2bd4b9 100644
--- a/drivers/connector/cn_proc.c
+++ b/drivers/connector/cn_proc.c
@@ -48,6 +48,21 @@  static DEFINE_PER_CPU(struct local_event, local_event) = {
 	.lock = INIT_LOCAL_LOCK(lock),
 };
 
+static int cn_filter(struct sock *dsk, struct sk_buff *skb, void *data)
+{
+	enum proc_cn_mcast_op mc_op;
+
+	if (!dsk)
+		return 0;
+
+	mc_op = ((struct proc_input *)(dsk->sk_user_data))->mcast_op;
+
+	if (mc_op == PROC_CN_MCAST_IGNORE)
+		return 1;
+
+	return 0;
+}
+
 static inline void send_msg(struct cn_msg *msg)
 {
 	local_lock(&local_event.lock);
@@ -61,7 +76,8 @@  static inline void send_msg(struct cn_msg *msg)
 	 *
 	 * If cn_netlink_send() fails, the data is not sent.
 	 */
-	cn_netlink_send(msg, 0, CN_IDX_PROC, GFP_NOWAIT);
+	cn_netlink_send_mult(msg, msg->len, 0, CN_IDX_PROC, GFP_NOWAIT,
+			     cn_filter, NULL);
 
 	local_unlock(&local_event.lock);
 }
@@ -346,11 +362,9 @@  static void cn_proc_ack(int err, int rcvd_seq, int rcvd_ack)
 static void cn_proc_mcast_ctl(struct cn_msg *msg,
 			      struct netlink_skb_parms *nsp)
 {
-	enum proc_cn_mcast_op *mc_op = NULL;
-	int err = 0;
-
-	if (msg->len != sizeof(*mc_op))
-		return;
+	enum proc_cn_mcast_op mc_op = 0, prev_mc_op = 0;
+	int err = 0, initial = 0;
+	struct sock *sk = NULL;
 
 	/* 
 	 * Events are reported with respect to the initial pid
@@ -367,13 +381,32 @@  static void cn_proc_mcast_ctl(struct cn_msg *msg,
 		goto out;
 	}
 
-	mc_op = (enum proc_cn_mcast_op *)msg->data;
-	switch (*mc_op) {
+	if (msg->len == sizeof(mc_op))
+		mc_op = *((enum proc_cn_mcast_op *)msg->data);
+	else
+		return;
+
+	if (nsp->sk) {
+		sk = nsp->sk;
+		if (sk->sk_user_data == NULL) {
+			sk->sk_user_data = kzalloc(sizeof(struct proc_input),
+						   GFP_KERNEL);
+			initial = 1;
+		} else {
+			prev_mc_op =
+			((struct proc_input *)(sk->sk_user_data))->mcast_op;
+		}
+		((struct proc_input *)(sk->sk_user_data))->mcast_op = mc_op;
+	}
+
+	switch (mc_op) {
 	case PROC_CN_MCAST_LISTEN:
-		atomic_inc(&proc_event_num_listeners);
+		if (initial || (prev_mc_op != PROC_CN_MCAST_LISTEN))
+			atomic_inc(&proc_event_num_listeners);
 		break;
 	case PROC_CN_MCAST_IGNORE:
-		atomic_dec(&proc_event_num_listeners);
+		if (!initial && (prev_mc_op != PROC_CN_MCAST_IGNORE))
+			atomic_dec(&proc_event_num_listeners);
 		break;
 	default:
 		err = EINVAL;
diff --git a/drivers/connector/connector.c b/drivers/connector/connector.c
index 48ec7ce6ecac..1b7851b1aa0f 100644
--- a/drivers/connector/connector.c
+++ b/drivers/connector/connector.c
@@ -59,7 +59,9 @@  static int cn_already_initialized;
  * both, or if both are zero then the group is looked up and sent there.
  */
 int cn_netlink_send_mult(struct cn_msg *msg, u16 len, u32 portid, u32 __group,
-	gfp_t gfp_mask)
+	gfp_t gfp_mask,
+	int (*filter)(struct sock *dsk, struct sk_buff *skb, void *data),
+	void *filter_data)
 {
 	struct cn_callback_entry *__cbq;
 	unsigned int size;
@@ -110,8 +112,9 @@  int cn_netlink_send_mult(struct cn_msg *msg, u16 len, u32 portid, u32 __group,
 	NETLINK_CB(skb).dst_group = group;
 
 	if (group)
-		return netlink_broadcast(dev->nls, skb, portid, group,
-					 gfp_mask);
+		return netlink_broadcast_filtered(dev->nls, skb, portid, group,
+						  gfp_mask, filter,
+						  (void *)filter_data);
 	return netlink_unicast(dev->nls, skb, portid,
 			!gfpflags_allow_blocking(gfp_mask));
 }
@@ -121,7 +124,8 @@  EXPORT_SYMBOL_GPL(cn_netlink_send_mult);
 int cn_netlink_send(struct cn_msg *msg, u32 portid, u32 __group,
 	gfp_t gfp_mask)
 {
-	return cn_netlink_send_mult(msg, msg->len, portid, __group, gfp_mask);
+	return cn_netlink_send_mult(msg, msg->len, portid, __group, gfp_mask,
+				    NULL, NULL);
 }
 EXPORT_SYMBOL_GPL(cn_netlink_send);
 
diff --git a/drivers/w1/w1_netlink.c b/drivers/w1/w1_netlink.c
index db110cc442b1..691978cddab7 100644
--- a/drivers/w1/w1_netlink.c
+++ b/drivers/w1/w1_netlink.c
@@ -65,7 +65,8 @@  static void w1_unref_block(struct w1_cb_block *block)
 		u16 len = w1_reply_len(block);
 		if (len) {
 			cn_netlink_send_mult(block->first_cn, len,
-				block->portid, 0, GFP_KERNEL);
+					     block->portid, 0,
+					     GFP_KERNEL, NULL, NULL);
 		}
 		kfree(block);
 	}
@@ -83,7 +84,8 @@  static void w1_reply_make_space(struct w1_cb_block *block, u16 space)
 {
 	u16 len = w1_reply_len(block);
 	if (len + space >= block->maxlen) {
-		cn_netlink_send_mult(block->first_cn, len, block->portid, 0, GFP_KERNEL);
+		cn_netlink_send_mult(block->first_cn, len, block->portid,
+				     0, GFP_KERNEL, NULL, NULL);
 		block->first_cn->len = 0;
 		block->cn = NULL;
 		block->msg = NULL;
diff --git a/include/linux/connector.h b/include/linux/connector.h
index 487350bb19c3..1336a5e7dd2f 100644
--- a/include/linux/connector.h
+++ b/include/linux/connector.h
@@ -96,7 +96,11 @@  void cn_del_callback(const struct cb_id *id);
  *
  * If there are no listeners for given group %-ESRCH can be returned.
  */
-int cn_netlink_send_mult(struct cn_msg *msg, u16 len, u32 portid, u32 group, gfp_t gfp_mask);
+int cn_netlink_send_mult(struct cn_msg *msg, u16 len, u32 portid,
+			 u32 group, gfp_t gfp_mask,
+			 int (*filter)(struct sock *dsk, struct sk_buff *skb,
+				       void *data),
+			 void *filter_data);
 
 /**
  * cn_netlink_send - Sends message to the specified groups.
diff --git a/include/uapi/linux/cn_proc.h b/include/uapi/linux/cn_proc.h
index db210625cee8..6a06fb424313 100644
--- a/include/uapi/linux/cn_proc.h
+++ b/include/uapi/linux/cn_proc.h
@@ -30,6 +30,30 @@  enum proc_cn_mcast_op {
 	PROC_CN_MCAST_IGNORE = 2
 };
 
+enum proc_cn_event {
+	/* Use successive bits so the enums can be used to record
+	 * sets of events as well
+	 */
+	PROC_EVENT_NONE = 0x00000000,
+	PROC_EVENT_FORK = 0x00000001,
+	PROC_EVENT_EXEC = 0x00000002,
+	PROC_EVENT_UID  = 0x00000004,
+	PROC_EVENT_GID  = 0x00000040,
+	PROC_EVENT_SID  = 0x00000080,
+	PROC_EVENT_PTRACE = 0x00000100,
+	PROC_EVENT_COMM = 0x00000200,
+	/* "next" should be 0x00000400 */
+	/* "last" is the last process event: exit,
+	 * while "next to last" is coredumping event
+	 */
+	PROC_EVENT_COREDUMP = 0x40000000,
+	PROC_EVENT_EXIT = 0x80000000
+};
+
+struct proc_input {
+	enum proc_cn_mcast_op mcast_op;
+};
+
 /*
  * From the user's point of view, the process
  * ID is the thread group ID and thread ID is the internal
@@ -44,24 +68,7 @@  enum proc_cn_mcast_op {
  */
 
 struct proc_event {
-	enum what {
-		/* Use successive bits so the enums can be used to record
-		 * sets of events as well
-		 */
-		PROC_EVENT_NONE = 0x00000000,
-		PROC_EVENT_FORK = 0x00000001,
-		PROC_EVENT_EXEC = 0x00000002,
-		PROC_EVENT_UID  = 0x00000004,
-		PROC_EVENT_GID  = 0x00000040,
-		PROC_EVENT_SID  = 0x00000080,
-		PROC_EVENT_PTRACE = 0x00000100,
-		PROC_EVENT_COMM = 0x00000200,
-		/* "next" should be 0x00000400 */
-		/* "last" is the last process event: exit,
-		 * while "next to last" is coredumping event */
-		PROC_EVENT_COREDUMP = 0x40000000,
-		PROC_EVENT_EXIT = 0x80000000
-	} what;
+	enum proc_cn_event what;
 	__u32 cpu;
 	__u64 __attribute__((aligned(8))) timestamp_ns;
 		/* Number of nano seconds since system boot */
diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
index 003c7e6ec9be..b311375b8c4c 100644
--- a/net/netlink/af_netlink.c
+++ b/net/netlink/af_netlink.c
@@ -63,6 +63,7 @@ 
 #include <linux/net_namespace.h>
 #include <linux/nospec.h>
 #include <linux/btf_ids.h>
+#include <linux/connector.h>
 
 #include <net/net_namespace.h>
 #include <net/netns/generic.h>
@@ -767,9 +768,14 @@  static int netlink_release(struct socket *sock)
 	/* must not acquire netlink_table_lock in any way again before unbind
 	 * and notifying genetlink is done as otherwise it might deadlock
 	 */
-	if (nlk->netlink_unbind) {
+	if (nlk->netlink_unbind && nlk->groups) {
 		int i;
-
+		if (sk->sk_protocol == NETLINK_CONNECTOR) {
+			if (test_bit(CN_IDX_PROC - 1, nlk->groups)) {
+				kfree(sk->sk_user_data);
+				sk->sk_user_data = NULL;
+			}
+		}
 		for (i = 0; i < nlk->ngroups; i++)
 			if (test_bit(i, nlk->groups))
 				nlk->netlink_unbind(sock_net(sk), i + 1);