Date: Tue, 4 Oct 2022 14:46:51 +0300 From: Andriy Gapon <avg@FreeBSD.org> To: Sean Bruno <sbruno@freebsd.org>, "net@FreeBSD.org" <net@FreeBSD.org> Subject: Re: in_pcbbind_setup: wrong condition regarding INP_REUSEPORT ? Message-ID: <9037ac3d-8d8e-2d7d-cbdb-996a53613aca@FreeBSD.org> In-Reply-To: <896ee089-27ad-c98f-6bf9-4b05caf778fd@freebsd.org> References: <ef09313c-14f4-01bc-b9c8-043f1c0ee830@FreeBSD.org> <896ee089-27ad-c98f-6bf9-4b05caf778fd@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?9037ac3d-8d8e-2d7d-cbdb-996a53613aca>