netdev - Re: Re: [PATCH v1 net] sctp: Fix null-ptr-deref in reuseport_add

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: <20240731190625.58651-1-kuniyu@amazon.com>
Date: Wed, 31 Jul 2024 12:06:25 -0700
From: Kuniyuki Iwashima <kuniyu@...zon.com>
To: <dmantipov@...dex.ru>
CC: <davem@...emloft.net>, <edumazet@...gle.com>, <kuba@...nel.org>,
	<kuni1840@...il.com>, <kuniyu@...zon.com>, <linux-sctp@...r.kernel.org>,
	<lucien.xin@...il.com>, <marcelo.leitner@...il.com>,
	<netdev@...r.kernel.org>, <pabeni@...hat.com>,
	<syzbot+e6979a5d2f10ecb700e4@...kaller.appspotmail.com>
Subject: Re: Re: [PATCH v1 net] sctp: Fix null-ptr-deref in reuseport_add_sock().

From: Dmitry Antipov <dmantipov@...dex.ru>
Date: Wed, 31 Jul 2024 10:05:37 +0300
> > What happens if two sockets matching each other reach here ?
> 
> Not sure.

If two sockets reach the reuseport_alloc() path, two identical
reuseport groups are created.

Then, in __sctp_rcv_lookup_endpoint(), incoming packets hit only
one socket group placed before the other in the hash bucket, and
the other socket no longer receive data and silently died from
userspace POV.

The suggested change papers over the problem.

reusport_add_sock() and reuseport_alloc() must be placed in the
same writer critical section.

> In general, an attempt to use rather external thing (struct
> sctp_hashbucket) to implement extra synchronization for reuse innards
> looks somewhat weird.

reuseport_lock is to synchronise operations within the same
reuseport group, but which socket should belong to which group
is out of scope.

That must be well-defined and synchronised at the upper level.

I'll post v2 with updated changelog.

Thanks!