[PATCH mptcp-next] sock: add sock_krfree_s helper

[PATCH mptcp-next] sock: add sock_krfree_s helper

[Geliang Tang]

This patch adds the rcu version of sock_kfree_s() helper, named
sock_krfree_s(), to free the memory using kfree_rcu_mightsleep()
and adjust the socket's option memory buffer.

This new helper is first used in MPTCP.

Signed-off-by: Geliang Tang <geliang@kernel.org>
---
 include/net/sock.h       |  1 +
 net/core/sock.c          | 14 +++++++++++---
 net/mptcp/pm_userspace.c |  3 +--
 3 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/include/net/sock.h b/include/net/sock.h
index e223102337c7..94f72f5c8754 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -1836,6 +1836,7 @@ void *sock_kmemdup(struct sock *sk, const void *src,
 		   int size, gfp_t priority);
 void sock_kfree_s(struct sock *sk, void *mem, int size);
 void sock_kzfree_s(struct sock *sk, void *mem, int size);
+void sock_krfree_s(struct sock *sk, void *mem, int size);
 void sk_send_sigurg(struct sock *sk);
 
 static inline void sock_replace_proto(struct sock *sk, struct proto *proto)
diff --git a/net/core/sock.c b/net/core/sock.c
index b64df2463300..f9ee0b44ab2a 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -2862,12 +2862,14 @@ EXPORT_SYMBOL(sock_kmemdup);
  * condition entirely.
  */
 static inline void __sock_kfree_s(struct sock *sk, void *mem, int size,
-				  const bool nullify)
+				  const bool nullify, const bool rcu)
 {
 	if (WARN_ON_ONCE(!mem))
 		return;
 	if (nullify)
 		kfree_sensitive(mem);
+	else if (rcu)
+		kfree_rcu_mightsleep(mem);
 	else
 		kfree(mem);
 	atomic_sub(size, &sk->sk_omem_alloc);
@@ -2875,16 +2877,22 @@ static inline void __sock_kfree_s(struct sock *sk, void *mem, int size,
 
 void sock_kfree_s(struct sock *sk, void *mem, int size)
 {
-	__sock_kfree_s(sk, mem, size, false);
+	__sock_kfree_s(sk, mem, size, false, false);
 }
 EXPORT_SYMBOL(sock_kfree_s);
 
 void sock_kzfree_s(struct sock *sk, void *mem, int size)
 {
-	__sock_kfree_s(sk, mem, size, true);
+	__sock_kfree_s(sk, mem, size, true, false);
 }
 EXPORT_SYMBOL(sock_kzfree_s);
 
+void sock_krfree_s(struct sock *sk, void *mem, int size)
+{
+	__sock_kfree_s(sk, mem, size, false, true);
+}
+EXPORT_SYMBOL(sock_krfree_s);
+
 /* It is almost wait_for_tcp_memory minus release_sock/lock_sock.
    I think, these locks should be removed for datagram sockets.
  */
diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c
index 1911fe1799fa..60ddf4889c51 100644
--- a/net/mptcp/pm_userspace.c
+++ b/net/mptcp/pm_userspace.c
@@ -337,11 +337,10 @@ int mptcp_pm_nl_remove_doit(struct sk_buff *skb, struct genl_info *info)
 
 	release_sock(sk);
 
-	kfree_rcu_mightsleep(match);
 	/* Adjust sk_omem_alloc like sock_kfree_s() does, to match
 	 * with allocation of this memory by sock_kmemdup()
 	 */
-	atomic_sub(sizeof(*match), &sk->sk_omem_alloc);
+	sock_krfree_s(sk, match, sizeof(*match));
 
 	err = 0;
 out:
-- 
2.43.0

Re: [PATCH mptcp-next] sock: add sock_krfree_s helper

[Matthieu Baerts]

Hi Geliang,

On 30/04/2025 11:09, Geliang Tang wrote:
> This patch adds the rcu version of sock_kfree_s() helper, named
> sock_krfree_s(), to free the memory using kfree_rcu_mightsleep()
> and adjust the socket's option memory buffer.
> 
> This new helper is first used in MPTCP.
> 
> Signed-off-by: Geliang Tang <geliang@kernel.org>
> ---
>  include/net/sock.h       |  1 +
>  net/core/sock.c          | 14 +++++++++++---
>  net/mptcp/pm_userspace.c |  3 +--
>  3 files changed, 13 insertions(+), 5 deletions(-)
> 
> diff --git a/include/net/sock.h b/include/net/sock.h
> index e223102337c7..94f72f5c8754 100644
> --- a/include/net/sock.h
> +++ b/include/net/sock.h
> @@ -1836,6 +1836,7 @@ void *sock_kmemdup(struct sock *sk, const void *src,
>  		   int size, gfp_t priority);
>  void sock_kfree_s(struct sock *sk, void *mem, int size);
>  void sock_kzfree_s(struct sock *sk, void *mem, int size);
> +void sock_krfree_s(struct sock *sk, void *mem, int size);
>  void sk_send_sigurg(struct sock *sk);
>  
>  static inline void sock_replace_proto(struct sock *sk, struct proto *proto)
> diff --git a/net/core/sock.c b/net/core/sock.c
> index b64df2463300..f9ee0b44ab2a 100644
> --- a/net/core/sock.c
> +++ b/net/core/sock.c
> @@ -2862,12 +2862,14 @@ EXPORT_SYMBOL(sock_kmemdup);
>   * condition entirely.
>   */
>  static inline void __sock_kfree_s(struct sock *sk, void *mem, int size,
> -				  const bool nullify)
> +				  const bool nullify, const bool rcu)
>  {
>  	if (WARN_ON_ONCE(!mem))
>  		return;
>  	if (nullify)
>  		kfree_sensitive(mem);
> +	else if (rcu)
> +		kfree_rcu_mightsleep(mem);

I'm not sure about that: in some cases, kfree_rcu() might be called.
kfree_rcu_mightsleep() is more an exception to me, I think. Also, it
looks like there are many other places where the free and atomic_sub()
is done "manually", that's not specific to MPTCP.

I don't know what Mat thinks about that, but I don't think this is
needed (or it should replace some code in different places in the kernel).

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.

Re: [PATCH mptcp-next] sock: add sock_krfree_s helper

[Geliang Tang]

Hi Matt,

On Wed, 2025-04-30 at 11:46 +0200, Matthieu Baerts wrote:
> Hi Geliang,
> 
> On 30/04/2025 11:09, Geliang Tang wrote:
> > This patch adds the rcu version of sock_kfree_s() helper, named
> > sock_krfree_s(), to free the memory using kfree_rcu_mightsleep()
> > and adjust the socket's option memory buffer.
> > 
> > This new helper is first used in MPTCP.
> > 
> > Signed-off-by: Geliang Tang <geliang@kernel.org>
> > ---
> >  include/net/sock.h       |  1 +
> >  net/core/sock.c          | 14 +++++++++++---
> >  net/mptcp/pm_userspace.c |  3 +--
> >  3 files changed, 13 insertions(+), 5 deletions(-)
> > 
> > diff --git a/include/net/sock.h b/include/net/sock.h
> > index e223102337c7..94f72f5c8754 100644
> > --- a/include/net/sock.h
> > +++ b/include/net/sock.h
> > @@ -1836,6 +1836,7 @@ void *sock_kmemdup(struct sock *sk, const
> > void *src,
> >  		   int size, gfp_t priority);
> >  void sock_kfree_s(struct sock *sk, void *mem, int size);
> >  void sock_kzfree_s(struct sock *sk, void *mem, int size);
> > +void sock_krfree_s(struct sock *sk, void *mem, int size);
> >  void sk_send_sigurg(struct sock *sk);
> >  
> >  static inline void sock_replace_proto(struct sock *sk, struct
> > proto *proto)
> > diff --git a/net/core/sock.c b/net/core/sock.c
> > index b64df2463300..f9ee0b44ab2a 100644
> > --- a/net/core/sock.c
> > +++ b/net/core/sock.c
> > @@ -2862,12 +2862,14 @@ EXPORT_SYMBOL(sock_kmemdup);
> >   * condition entirely.
> >   */
> >  static inline void __sock_kfree_s(struct sock *sk, void *mem, int
> > size,
> > -				  const bool nullify)
> > +				  const bool nullify, const bool
> > rcu)
> >  {
> >  	if (WARN_ON_ONCE(!mem))
> >  		return;
> >  	if (nullify)
> >  		kfree_sensitive(mem);
> > +	else if (rcu)
> > +		kfree_rcu_mightsleep(mem);
> 
> I'm not sure about that: in some cases, kfree_rcu() might be called.
> kfree_rcu_mightsleep() is more an exception to me, I think. Also, it
> looks like there are many other places where the free and
> atomic_sub()
> is done "manually", that's not specific to MPTCP.
> 
> I don't know what Mat thinks about that, but I don't think this is
> needed (or it should replace some code in different places in the
> kernel).

Indeed, thanks for your reminder. I didn't notice that there are many
other places that use free() and atomic_sub() too. I will reconsider
whether to drop this patch or replace some code in different places.

Thanks,
-Geliang

> 
> Cheers,
> Matt