netdev - Re: [PATCH v1 net-next 2/7] tcp: Save __module_get() for TIME

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAAVpQUC1yEB-VqKr0qPCmGEKHkfSyGt-bZn7bdEgCHe6c3FH+g@mail.gmail.com>
Date: Thu, 21 Aug 2025 10:16:26 -0700
From: Kuniyuki Iwashima <kuniyu@...gle.com>
To: Eric Dumazet <edumazet@...gle.com>
Cc: "David S. Miller" <davem@...emloft.net>, Jakub Kicinski <kuba@...nel.org>, 
	Paolo Abeni <pabeni@...hat.com>, Neal Cardwell <ncardwell@...gle.com>, 
	David Ahern <dsahern@...nel.org>, Simon Horman <horms@...nel.org>, 
	Kuniyuki Iwashima <kuni1840@...il.com>, netdev@...r.kernel.org, 
	Kuniyuki Iwashima <kuniyu@...zon.com>
Subject: Re: [PATCH v1 net-next 2/7] tcp: Save __module_get() for TIME_WAIT sockets.

On Wed, Aug 20, 2025 at 11:46 PM Eric Dumazet <edumazet@...gle.com> wrote:
>
> On Wed, Aug 20, 2025 at 11:16 PM Kuniyuki Iwashima <kuniyu@...gle.com> wrote:
> >
> > From: Kuniyuki Iwashima <kuniyu@...zon.com>
> >
> > __module_get() in inet_twsk_alloc() was necessary to prevent
> > unloading tw->tw_prot, which is used in inet_twsk_free().
> >
> > DCCP has gone, and TCP is built-in, so the pair is no longer needed.
> >
> > ULPs also do not need it because
> >
> >  * kTLS and XFRM_ESPINTCP restore sk_prot before close()
> >  * MPTCP is built-in
> >  * SMC uses TCP as is
> >
> > , but using tw_prot without module_get() would be error prone to
> > future ULP addition.
> >
> > Now we can use kfree() without the slab cache pointer thanks to SLUB.
>
> Right, but kmem_cache_free() has extra debug checks (SLAB_CONSISTENCY_CHECKS):
> we check the object was indeed allocated from a precise cache.
>
> I would prefer leaving this in place.
>
> Such a conversion could be done globally if you think about it, no
> need for hundreds of patches.
>
> static inline void kmem_cache_free(struct kmem_cache *s, void *x)
> {
>       kfree(x);
> }
>
>
> >
> > Let's use kfree() in inet_twsk_free() and remove 2 atomic ops
> > for each connection.
> >
>
> Where are you seeing the atomic ops exactly ?
>
> TCP is builtin, so .owner is NULL.

Ah exactly, I think I was confused with DCCP.
I'll drop this patch in v2.

Thanks!