netdev - Re: [PATCH v3 net] dccp/tcp: Unhash sk from ehash for tb2 alloc failure after check

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: <CANn89i+Hg174E+stnX7dSb5PzYDk_07biqZL1FSrNFaT5BC2qw@mail.gmail.com>
Date: Wed, 14 Feb 2024 20:31:46 +0100
From: Eric Dumazet <edumazet@...gle.com>
To: Kuniyuki Iwashima <kuniyu@...zon.com>
Cc: "David S. Miller" <davem@...emloft.net>, Jakub Kicinski <kuba@...nel.org>, 
	Paolo Abeni <pabeni@...hat.com>, David Ahern <dsahern@...nel.org>, 
	Joanne Koong <joannelkoong@...il.com>, Kuniyuki Iwashima <kuni1840@...il.com>, netdev@...r.kernel.org, 
	syzkaller <syzkaller@...glegroups.com>
Subject: Re: [PATCH v3 net] dccp/tcp: Unhash sk from ehash for tb2 alloc
 failure after check_estalblished().

On Wed, Feb 14, 2024 at 8:13 PM Kuniyuki Iwashima <kuniyu@...zon.com> wrote:
>
> syzkaller reported a warning [0] in inet_csk_destroy_sock() with no
> repro.
>
>   WARN_ON(inet_sk(sk)->inet_num && !inet_csk(sk)->icsk_bind_hash);
>
> However, the syzkaller's log hinted that connect() failed just before
> the warning due to FAULT_INJECTION.  [1]
>
> When connect() is called for an unbound socket, we search for an
> available ephemeral port.  If a bhash bucket exists for the port, we
> call __inet_check_established() or __inet6_check_established() to check
> if the bucket is reusable.
>
> If reusable, we add the socket into ehash and set inet_sk(sk)->inet_num.
>
> Later, we look up the corresponding bhash2 bucket and try to allocate
> it if it does not exist.
>
> Although it rarely occurs in real use, if the allocation fails, we must
> revert the changes by check_established().  Otherwise, an unconnected
> socket could illegally occupy an ehash entry.
>
> Note that we do not put tw back into ehash because sk might have
> already responded to a packet for tw and it would be better to free
> tw earlier under such memory presure.
>
> [0]:
>
> Reported-by: syzkaller <syzkaller@...glegroups.com>
> Fixes: 28044fc1d495 ("net: Add a bhash2 table hashed by port and address")
> Signed-off-by: Kuniyuki Iwashima <kuniyu@...zon.com>
> ---

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