netdev - Re: [PATCH net] net: inet_csk: Fix so_reuseport bind-address cache in tb->fast*

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: <20200519.153706.1484434976528519598.davem@davemloft.net>
Date:   Tue, 19 May 2020 15:37:06 -0700 (PDT)
From:   David Miller <davem@...emloft.net>
To:     kafai@...com
Cc:     netdev@...r.kernel.org, kernel-team@...com, jbacik@...com
Subject: Re: [PATCH net] net: inet_csk: Fix so_reuseport bind-address cache
 in tb->fast*

From: Martin KaFai Lau <kafai@...com>
Date: Mon, 18 May 2020 17:13:34 -0700

> The commit 637bc8bbe6c0 ("inet: reset tb->fastreuseport when adding a reuseport sk")
> added a bind-address cache in tb->fast*.  The tb->fast* caches the address
> of a sk which has successfully been binded with SO_REUSEPORT ON.  The idea
> is to avoid the expensive conflict search in inet_csk_bind_conflict().
> 
> There is an issue with wildcard matching where sk_reuseport_match() should
> have returned false but it is currently returning true.  It ends up
> hiding bind conflict.  For example,
> 
> bind("[::1]:443"); /* without SO_REUSEPORT. Succeed. */
> bind("[::2]:443"); /* with    SO_REUSEPORT. Succeed. */
> bind("[::]:443");  /* with    SO_REUSEPORT. Still Succeed where it shouldn't */
> 
> The last bind("[::]:443") with SO_REUSEPORT on should have failed because
> it should have a conflict with the very first bind("[::1]:443") which
> has SO_REUSEPORT off.  However, the address "[::2]" is cached in
> tb->fast* in the second bind. In the last bind, the sk_reuseport_match()
> returns true because the binding sk's wildcard addr "[::]" matches with
> the "[::2]" cached in tb->fast*.
> 
> The correct bind conflict is reported by removing the second
> bind such that tb->fast* cache is not involved and forces the
> bind("[::]:443") to go through the inet_csk_bind_conflict():
> 
> bind("[::1]:443"); /* without SO_REUSEPORT. Succeed. */
> bind("[::]:443");  /* with    SO_REUSEPORT. -EADDRINUSE */
> 
> The expected behavior for sk_reuseport_match() is, it should only allow
> the "cached" tb->fast* address to be used as a wildcard match but not
> the address of the binding sk.  To do that, the current
> "bool match_wildcard" arg is split into
> "bool match_sk1_wildcard" and "bool match_sk2_wildcard".
> 
> This change only affects the sk_reuseport_match() which is only
> used by inet_csk (e.g. TCP).
> The other use cases are calling inet_rcv_saddr_equal() and
> this patch makes it pass the same "match_wildcard" arg twice to
> the "ipv[46]_rcv_saddr_equal(..., match_wildcard, match_wildcard)".
> 
> Cc: Josef Bacik <jbacik@...com>
> Fixes: 637bc8bbe6c0 ("inet: reset tb->fastreuseport when adding a reuseport sk")
> Signed-off-by: Martin KaFai Lau <kafai@...com>

Applied and queued up for -stable, thanks.