netdev - Re: [PATCH v1 net-next 3/3] tcp: Remove redundant sk_unhashed() in inet

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] [day] [month] [year] [list]

Message-ID: <CANn89iKdzw=22NTAT-uGhwNThvA7iZT6WbrdiwN7jQpLOXE3qw@mail.gmail.com>
Date: Fri, 19 Sep 2025 13:57:33 -0700
From: Eric Dumazet <edumazet@...gle.com>
To: Kuniyuki Iwashima <kuniyu@...gle.com>
Cc: Neal Cardwell <ncardwell@...gle.com>, "David S. Miller" <davem@...emloft.net>, 
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>, Simon Horman <horms@...nel.org>, 
	Xuanqiang Luo <xuanqiang.luo@...ux.dev>, Kuniyuki Iwashima <kuni1840@...il.com>, netdev@...r.kernel.org
Subject: Re: [PATCH v1 net-next 3/3] tcp: Remove redundant sk_unhashed() in inet_unhash().

On Fri, Sep 19, 2025 at 1:37 AM Kuniyuki Iwashima <kuniyu@...gle.com> wrote:
>
> inet_unhash() checks sk_unhashed() twice at the entry and after locking
> ehash/lhash bucket.
>
> The former was somehow added redundantly by commit 4f9bf2a2f5aa ("tcp:
> Don't acquire inet_listen_hashbucket::lock with disabled BH.").
>
> inet_unhash() is called for the full socket from 4 places, and it is
> always under lock_sock() or the socket is not yet published to other
> threads:
>
>   1. __sk_prot_rehash()
>      -> called from inet_sk_reselect_saddr(), which has
>         lockdep_sock_is_held()
>
>   2. sk_common_release()
>      -> called when inet_create() or inet6_create() fail, then the
>         socket is not yet published
>
>   3. tcp_set_state()
>      -> calls tcp_call_bpf_2arg(), and tcp_call_bpf() has
>         sock_owned_by_me()
>
>   4. inet_ctl_sock_create()
>      -> creates a kernel socket and unhashes it immediately, but TCP
>         socket is not hashed in sock_create_kern() (only SOCK_RAW is)
>
> So we do not need to check sk_unhashed() twice before/after ehash/lhash
> lock in inet_unhash().
>
> Let's remove the 2nd one.
>
> Signed-off-by: Kuniyuki Iwashima <kuniyu@...gle.com>

Reviewed-by: Eric Dumazet <edumazet@...gle.com>