On 04/10/2022 14:37, Sean Bruno wrote:
On 10/3/22 04:14, Andriy Gapon wrote:
I must admit that the condition in question is fairly long and non-trivial and
I cannot decipher it, but these two lines look wrong to me:
(t->inp_flags2 & INP_REUSEPORT) ||
(t->inp_flags2 & INP_REUSEPORT_LB) == 0)
&&
I'd expect that the check would be symmetric with respect to INP_REUSEPORT and
INP_REUSEPORT_LB.
The problem seems to come from 1a43cff92a20d / r334719 / D11003.
I think you are pointing at this absurd conditional?
https://cgit.freebsd.org/src/tree/sys/netinet/in_pcb.c#n1049
Besides the twisted logic, what problem are you trying to solve?
Yes, that conditional.
I pointed out the part of it that does not make sense to me.
Also, in my tests SO_REUSEPORT does not actually allow to share a port.
Test scenario:
- create a UDP socket
- setsockopt(SO_REUSEPORT)
- bind the socket to a port and wild card address
- success
- now repeat the previous steps with the same port *under a different user id*
- bind fails
I wonder if the following would be a correct change.
diff --git a/sys/netinet/in_pcb.c b/sys/netinet/in_pcb.c
index d9247f50d32b..f5e6e3932a96 100644
--- a/sys/netinet/in_pcb.c
+++ b/sys/netinet/in_pcb.c
@@ -1003,6 +1003,7 @@ in_pcbbind_setup(struct inpcb *inp, struct sockaddr *nam,
in_addr_t *laddrp,
/*
* XXX
* This entire block sorely needs a rewrite.
+ * And a good comment describing the rationale behind the conditions.
*/
if (t &&
((inp->inp_flags2 & INP_BINDMULTI) == 0) &&
@@ -1011,8 +1012,7 @@ in_pcbbind_setup(struct inpcb *inp, struct sockaddr *nam,
in_addr_t *laddrp,
ntohl(t->inp_faddr.s_addr) == INADDR_ANY)
&&
(ntohl(sin->sin_addr.s_addr) != INADDR_ANY
||
ntohl(t->inp_laddr.s_addr) != INADDR_ANY ||
- (t->inp_flags2 & INP_REUSEPORT) ||
- (t->inp_flags2 & INP_REUSEPORT_LB) == 0) &&
+ (t->inp_flags2 & (INP_REUSEPORT |
INP_REUSEPORT_LB)) == 0) &&
(inp->inp_cred->cr_uid !=
t->inp_cred->cr_uid))
return (EADDRINUSE);
--
Andriy Gapon
