Re: in_pcbbind_setup: wrong condition regarding INP_REUSEPORT ?
- Reply: Andriy Gapon : "UDP port re-use [Was: in_pcbbind_setup: wrong condition regarding INP_REUSEPORT ?]"
- Reply: Gleb Smirnoff : "Re: in_pcbbind_setup: wrong condition regarding INP_REUSEPORT ?"
- Reply: Mark Johnston : "Re: in_pcbbind_setup: wrong condition regarding INP_REUSEPORT ?"
- In reply to: Sean Bruno : "Re: in_pcbbind_setup: wrong condition regarding INP_REUSEPORT ?"
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
Date: Tue, 04 Oct 2022 11:46:51 UTC
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