On Thu, 5 Dec 2024 15:20:48 GMT Mark Johnston <ma...@freebsd.org> wrote:
This commit breaks buildkernel: [...] --- in6_pcb.o --- /usr/src/sys/netinet6/in6_pcb.c:301:20: error: unused variable 'pcbinfo' [-Werror,-Wunused-variable] 301 | struct inpcbinfo *pcbinfo = inp->inp_pcbinfo; > The branch main has been updated by markj: > > URL: > https://cgit.FreeBSD.org/src/commit/?id=01f8ce83242d7a8e599cf6a78b6277161d79edd4 > > commit 01f8ce83242d7a8e599cf6a78b6277161d79edd4 > Author: Mark Johnston <ma...@freebsd.org> > AuthorDate: 2024-12-05 15:00:13 +0000 > Commit: Mark Johnston <ma...@freebsd.org> > CommitDate: 2024-12-05 15:19:57 +0000 > > inpcb: Factor out parts of in6_pcbbind() and in_pcbbind_setup() > > A large portion of these functions just determines whether the inpcb can > bind to the address/port. This portion has no side effects, so is a > good candidate to move into its own helper function. This patch does > so, making the callers less complicated and reducing indentation. > > While moving this code, also make some changes: > - Load socket options (SO_REUSEADDR etc.) only once. There is nothing > preventing another thread from toggling the socket options, so make > this function easier to reason about by avoiding races. > - When checking whether the bind address is an interface address, make a > separate sockaddr rather than temporarily modifying the one passed to > in_pcbbind(). > > Reviewed by: ae, glebius > MFC after: 1 month > Sponsored by: Klara, Inc. > Sponsored by: Stormshield > Differential Revision: https://reviews.freebsd.org/D47590 > --- > sys/netinet/in_pcb.c | 172 ++++++++++++++++++-------------- > sys/netinet6/in6_pcb.c | 262 > +++++++++++++++++++++++++++---------------------- > 2 files changed, 242 insertions(+), 192 deletions(-) > > diff --git a/sys/netinet/in_pcb.c b/sys/netinet/in_pcb.c > index 7c881817ac6e..94fb406bcd2b 100644 > --- a/sys/netinet/in_pcb.c > +++ b/sys/netinet/in_pcb.c > @@ -861,6 +861,93 @@ in_pcb_lport(struct inpcb *inp, struct in_addr *laddrp, > u_short > *lportp, #endif /* INET || INET6 */ > > #ifdef INET > +/* > + * Determine whether the inpcb can be bound to the specified address/port > tuple. > + */ > +static int > +in_pcbbind_avail(struct inpcb *inp, const struct in_addr laddr, > + const u_short lport, int sooptions, int lookupflags, struct ucred *cred) > +{ > + int reuseport, reuseport_lb; > + > + INP_LOCK_ASSERT(inp); > + INP_HASH_LOCK_ASSERT(inp->inp_pcbinfo); > + > + reuseport = (sooptions & SO_REUSEPORT); > + reuseport_lb = (sooptions & SO_REUSEPORT_LB); > + > + if (IN_MULTICAST(ntohl(laddr.s_addr))) { > + /* > + * Treat SO_REUSEADDR as SO_REUSEPORT for multicast; > + * allow complete duplication of binding if > + * SO_REUSEPORT is set, or if SO_REUSEADDR is set > + * and a multicast address is bound on both > + * new and duplicated sockets. > + */ > + if ((sooptions & (SO_REUSEADDR | SO_REUSEPORT)) != 0) > + reuseport = SO_REUSEADDR | SO_REUSEPORT; > + /* > + * XXX: How to deal with SO_REUSEPORT_LB here? > + * Treat same as SO_REUSEPORT for now. > + */ > + if ((sooptions & (SO_REUSEADDR | SO_REUSEPORT_LB)) != 0) > + reuseport_lb = SO_REUSEADDR | SO_REUSEPORT_LB; > + } else if (!in_nullhost(laddr)) { > + struct sockaddr_in sin; > + > + memset(&sin, 0, sizeof(sin)); > + sin.sin_family = AF_INET; > + sin.sin_len = sizeof(sin); > + sin.sin_addr = laddr; > + > + /* > + * Is the address a local IP address? > + * If INP_BINDANY is set, then the socket may be bound > + * to any endpoint address, local or not. > + */ > + if ((inp->inp_flags & INP_BINDANY) == 0 && > + ifa_ifwithaddr_check((const struct sockaddr *)&sin) == 0) > + return (EADDRNOTAVAIL); > + } > + > + if (lport != 0) { > + struct inpcb *t; > + > + if (ntohs(lport) <= V_ipport_reservedhigh && > + ntohs(lport) >= V_ipport_reservedlow && > + priv_check_cred(cred, PRIV_NETINET_RESERVEDPORT)) > + return (EACCES); > + > + if (!IN_MULTICAST(ntohl(laddr.s_addr)) && > + priv_check_cred(inp->inp_cred, PRIV_NETINET_REUSEPORT) != > 0) { > + t = in_pcblookup_local(inp->inp_pcbinfo, laddr, lport, > + INPLOOKUP_WILDCARD, cred); > + if (t != NULL && > + (inp->inp_socket->so_type != SOCK_STREAM || > + in_nullhost(t->inp_faddr)) && > + (!in_nullhost(laddr) || > + !in_nullhost(t->inp_laddr) || > + (t->inp_socket->so_options & SO_REUSEPORT) || > + (t->inp_socket->so_options & SO_REUSEPORT_LB) == > 0) && > + (inp->inp_cred->cr_uid != t->inp_cred->cr_uid)) > + return (EADDRINUSE); > + } > + t = in_pcblookup_local(inp->inp_pcbinfo, laddr, lport, > + lookupflags, cred); > + if (t != NULL && ((reuseport | reuseport_lb) & > + t->inp_socket->so_options) == 0) { > +#ifdef INET6 > + if (!in_nullhost(laddr) || > + !in_nullhost(t->inp_laddr) || > + (inp->inp_vflag & INP_IPV6PROTO) == 0 || > + (t->inp_vflag & INP_IPV6PROTO) == 0) > +#endif > + return (EADDRINUSE); > + } > + } > + return (0); > +} > + > /* > * Set up a bind operation on a PCB, performing port allocation > * as required, but do not actually modify the PCB. Callers can > @@ -878,15 +965,9 @@ in_pcbbind_setup(struct inpcb *inp, struct sockaddr_in > *sin, > in_addr_t *laddrp, struct inpcbinfo *pcbinfo = inp->inp_pcbinfo; > struct in_addr laddr; > u_short lport = 0; > - int lookupflags = 0, reuseport = (so->so_options & SO_REUSEPORT); > + int lookupflags, sooptions; > int error; > > - /* > - * XXX: Maybe we could let SO_REUSEPORT_LB set SO_REUSEPORT bit here > - * so that we don't have to add to the (already messy) code below. > - */ > - int reuseport_lb = (so->so_options & SO_REUSEPORT_LB); > - > /* > * No state changes, so read locks are sufficient here. > */ > @@ -896,7 +977,10 @@ in_pcbbind_setup(struct inpcb *inp, struct sockaddr_in > *sin, > in_addr_t *laddrp, laddr.s_addr = *laddrp; > if (sin != NULL && laddr.s_addr != INADDR_ANY) > return (EINVAL); > - if ((so->so_options & (SO_REUSEADDR|SO_REUSEPORT|SO_REUSEPORT_LB)) == 0) > + > + lookupflags = 0; > + sooptions = atomic_load_int(&so->so_options); > + if ((sooptions & (SO_REUSEADDR | SO_REUSEPORT | SO_REUSEPORT_LB)) == 0) > lookupflags = INPLOOKUP_WILDCARD; > if (sin == NULL) { > if ((error = prison_local_ip4(cred, &laddr)) != 0) > @@ -918,73 +1002,11 @@ in_pcbbind_setup(struct inpcb *inp, struct sockaddr_in > *sin, > in_addr_t *laddrp, } > laddr = sin->sin_addr; > > - /* NB: lport is left as 0 if the port isn't being changed. */ > - if (IN_MULTICAST(ntohl(laddr.s_addr))) { > - /* > - * Treat SO_REUSEADDR as SO_REUSEPORT for multicast; > - * allow complete duplication of binding if > - * SO_REUSEPORT is set, or if SO_REUSEADDR is set > - * and a multicast address is bound on both > - * new and duplicated sockets. > - */ > - if ((so->so_options & (SO_REUSEADDR|SO_REUSEPORT)) != 0) > - reuseport = SO_REUSEADDR|SO_REUSEPORT; > - /* > - * XXX: How to deal with SO_REUSEPORT_LB here? > - * Treat same as SO_REUSEPORT for now. > - */ > - if ((so->so_options & > - (SO_REUSEADDR|SO_REUSEPORT_LB)) != 0) > - reuseport_lb = SO_REUSEADDR|SO_REUSEPORT_LB; > - } else if (!in_nullhost(laddr)) { > - sin->sin_port = 0; /* yech... */ > - bzero(&sin->sin_zero, sizeof(sin->sin_zero)); > - /* > - * Is the address a local IP address? > - * If INP_BINDANY is set, then the socket may be bound > - * to any endpoint address, local or not. > - */ > - if ((inp->inp_flags & INP_BINDANY) == 0 && > - ifa_ifwithaddr_check( > - (const struct sockaddr *)sin) == 0) > - return (EADDRNOTAVAIL); > - } > - if (lport) { > - struct inpcb *t; > - > - if (ntohs(lport) <= V_ipport_reservedhigh && > - ntohs(lport) >= V_ipport_reservedlow && > - priv_check_cred(cred, PRIV_NETINET_RESERVEDPORT)) > - return (EACCES); > - > - if (!IN_MULTICAST(ntohl(laddr.s_addr)) && > - priv_check_cred(inp->inp_cred, > PRIV_NETINET_REUSEPORT) != > 0) { > - t = in_pcblookup_local(pcbinfo, laddr, lport, > - INPLOOKUP_WILDCARD, cred); > - if (t != NULL && > - (so->so_type != SOCK_STREAM || > - in_nullhost(t->inp_faddr)) && > - (!in_nullhost(laddr) || > - !in_nullhost(t->inp_laddr) || > - (t->inp_socket->so_options & SO_REUSEPORT) > || > - (t->inp_socket->so_options & > SO_REUSEPORT_LB) == > 0) && > - (inp->inp_cred->cr_uid != > - t->inp_cred->cr_uid)) > - return (EADDRINUSE); > - } > - t = in_pcblookup_local(pcbinfo, laddr, lport, > - lookupflags, cred); > - if (t != NULL && ((reuseport | reuseport_lb) & > - t->inp_socket->so_options) == 0) { > -#ifdef INET6 > - if (!in_nullhost(laddr) || > - !in_nullhost(t->inp_laddr) || > - (inp->inp_vflag & INP_IPV6PROTO) == 0 || > - (t->inp_vflag & INP_IPV6PROTO) == 0) > -#endif > - return (EADDRINUSE); > - } > - } > + /* See if this address/port combo is available. */ > + error = in_pcbbind_avail(inp, laddr, lport, sooptions, > + lookupflags, cred); > + if (error != 0) > + return (error); > } > if (*lportp != 0) > lport = *lportp; > diff --git a/sys/netinet6/in6_pcb.c b/sys/netinet6/in6_pcb.c > index 639ec68e19e2..6b5a9d3706be 100644 > --- a/sys/netinet6/in6_pcb.c > +++ b/sys/netinet6/in6_pcb.c > @@ -160,27 +160,157 @@ in6_pcbsetport(struct in6_addr *laddr, struct inpcb > *inp, struct > ucred *cred) return (0); > } > > +/* > + * Determine whether the inpcb can be bound to the specified address/port > tuple. > + */ > +static int > +in6_pcbbind_avail(struct inpcb *inp, const struct sockaddr_in6 *sin6, > + int sooptions, int lookupflags, struct ucred *cred) > +{ > + const struct in6_addr *laddr; > + int reuseport, reuseport_lb; > + u_short lport; > + > + INP_LOCK_ASSERT(inp); > + INP_HASH_LOCK_ASSERT(inp->inp_pcbinfo); > + > + laddr = &sin6->sin6_addr; > + lport = sin6->sin6_port; > + > + reuseport = (sooptions & SO_REUSEPORT); > + reuseport_lb = (sooptions & SO_REUSEPORT_LB); > + > + if (IN6_IS_ADDR_MULTICAST(laddr)) { > + /* > + * Treat SO_REUSEADDR as SO_REUSEPORT for multicast; > + * allow compepte duplication of binding if > + * SO_REUSEPORT is set, or if SO_REUSEADDR is set > + * and a multicast address is bound on both > + * new and duplicated sockets. > + */ > + if ((sooptions & (SO_REUSEADDR | SO_REUSEPORT)) != 0) > + reuseport = SO_REUSEADDR | SO_REUSEPORT; > + /* > + * XXX: How to deal with SO_REUSEPORT_LB here? > + * Treat same as SO_REUSEPORT for now. > + */ > + if ((sooptions & (SO_REUSEADDR | SO_REUSEPORT_LB)) != 0) > + reuseport_lb = SO_REUSEADDR | SO_REUSEPORT_LB; > + } else if (!IN6_IS_ADDR_UNSPECIFIED(laddr)) { > + struct sockaddr_in6 sin6; > + struct epoch_tracker et; > + struct ifaddr *ifa; > + > + memset(&sin6, 0, sizeof(sin6)); > + sin6.sin6_family = AF_INET6; > + sin6.sin6_len = sizeof(sin6); > + sin6.sin6_addr = *laddr; > + > + NET_EPOCH_ENTER(et); > + if ((ifa = ifa_ifwithaddr((const struct sockaddr *)&sin6)) == > + NULL && (inp->inp_flags & INP_BINDANY) == 0) { > + NET_EPOCH_EXIT(et); > + return (EADDRNOTAVAIL); > + } > + > + /* > + * XXX: bind to an anycast address might accidentally > + * cause sending a packet with anycast source address. > + * We should allow to bind to a deprecated address, since > + * the application dares to use it. > + */ > + if (ifa != NULL && > + ((struct in6_ifaddr *)ifa)->ia6_flags & > + (IN6_IFF_ANYCAST | IN6_IFF_NOTREADY | IN6_IFF_DETACHED)) { > + NET_EPOCH_EXIT(et); > + return (EADDRNOTAVAIL); > + } > + NET_EPOCH_EXIT(et); > + } > + > + if (lport != 0) { > + struct inpcb *t; > + > + if (ntohs(lport) <= V_ipport_reservedhigh && > + ntohs(lport) >= V_ipport_reservedlow && > + priv_check_cred(cred, PRIV_NETINET_RESERVEDPORT)) > + return (EACCES); > + > + if (!IN6_IS_ADDR_MULTICAST(laddr) && > + priv_check_cred(inp->inp_cred, PRIV_NETINET_REUSEPORT) != > + 0) { > + t = in6_pcblookup_local(inp->inp_pcbinfo, laddr, lport, > + INPLOOKUP_WILDCARD, cred); > + if (t != NULL && > + (inp->inp_socket->so_type != SOCK_STREAM || > + IN6_IS_ADDR_UNSPECIFIED(&t->in6p_faddr)) && > + (!IN6_IS_ADDR_UNSPECIFIED(laddr) || > + !IN6_IS_ADDR_UNSPECIFIED(&t->in6p_laddr) || > + (t->inp_socket->so_options & SO_REUSEPORT) || > + (t->inp_socket->so_options & SO_REUSEPORT_LB) == > 0) && > + (inp->inp_cred->cr_uid != t->inp_cred->cr_uid)) > + return (EADDRINUSE); > + > +#ifdef INET > + if ((inp->inp_flags & IN6P_IPV6_V6ONLY) == 0 && > + IN6_IS_ADDR_UNSPECIFIED(laddr)) { > + struct sockaddr_in sin; > + > + in6_sin6_2_sin(&sin, sin6); > + t = in_pcblookup_local(inp->inp_pcbinfo, > + sin.sin_addr, lport, INPLOOKUP_WILDCARD, > + cred); > + if (t != NULL && > + (inp->inp_socket->so_type != SOCK_STREAM || > + in_nullhost(t->inp_faddr)) && > + (inp->inp_cred->cr_uid != > + t->inp_cred->cr_uid)) > + return (EADDRINUSE); > + } > +#endif > + } > + t = in6_pcblookup_local(inp->inp_pcbinfo, laddr, lport, > + lookupflags, cred); > + if (t != NULL && ((reuseport | reuseport_lb) & > + t->inp_socket->so_options) == 0) > + return (EADDRINUSE); > +#ifdef INET > + if ((inp->inp_flags & IN6P_IPV6_V6ONLY) == 0 && > + IN6_IS_ADDR_UNSPECIFIED(laddr)) { > + struct sockaddr_in sin; > + > + in6_sin6_2_sin(&sin, sin6); > + t = in_pcblookup_local(inp->inp_pcbinfo, sin.sin_addr, > + lport, lookupflags, cred); > + if (t != NULL && ((reuseport | reuseport_lb) & > + t->inp_socket->so_options) == 0 && > + (!in_nullhost(t->inp_laddr) || > + (t->inp_vflag & INP_IPV6PROTO) != 0)) { > + return (EADDRINUSE); > + } > + } > +#endif > + } > + return (0); > +} > + > int > in6_pcbbind(struct inpcb *inp, struct sockaddr_in6 *sin6, struct ucred *cred) > { > struct socket *so = inp->inp_socket; > struct inpcbinfo *pcbinfo = inp->inp_pcbinfo; > u_short lport = 0; > - int error, lookupflags = 0; > - int reuseport = (so->so_options & SO_REUSEPORT); > - > - /* > - * XXX: Maybe we could let SO_REUSEPORT_LB set SO_REUSEPORT bit here > - * so that we don't have to add to the (already messy) code below. > - */ > - int reuseport_lb = (so->so_options & SO_REUSEPORT_LB); > + int error, lookupflags, sooptions; > > INP_WLOCK_ASSERT(inp); > INP_HASH_WLOCK_ASSERT(pcbinfo); > > if (inp->inp_lport || !IN6_IS_ADDR_UNSPECIFIED(&inp->in6p_laddr)) > return (EINVAL); > - if ((so->so_options & (SO_REUSEADDR|SO_REUSEPORT|SO_REUSEPORT_LB)) == 0) > + > + lookupflags = 0; > + sooptions = atomic_load_int(&so->so_options); > + if ((sooptions & (SO_REUSEADDR | SO_REUSEPORT | SO_REUSEPORT_LB)) == 0) > lookupflags = INPLOOKUP_WILDCARD; > if (sin6 == NULL) { > if ((error = prison_local_ip6(cred, &inp->in6p_laddr, > @@ -199,115 +329,13 @@ in6_pcbbind(struct inpcb *inp, struct sockaddr_in6 > *sin6, struct > ucred *cred) ((inp->inp_flags & IN6P_IPV6_V6ONLY) != 0))) != 0) > return (error); > > - lport = sin6->sin6_port; > - if (IN6_IS_ADDR_MULTICAST(&sin6->sin6_addr)) { > - /* > - * Treat SO_REUSEADDR as SO_REUSEPORT for multicast; > - * allow compepte duplication of binding if > - * SO_REUSEPORT is set, or if SO_REUSEADDR is set > - * and a multicast address is bound on both > - * new and duplicated sockets. > - */ > - if ((so->so_options & (SO_REUSEADDR|SO_REUSEPORT)) != 0) > - reuseport = SO_REUSEADDR|SO_REUSEPORT; > - /* > - * XXX: How to deal with SO_REUSEPORT_LB here? > - * Treat same as SO_REUSEPORT for now. > - */ > - if ((so->so_options & > - (SO_REUSEADDR|SO_REUSEPORT_LB)) != 0) > - reuseport_lb = SO_REUSEADDR|SO_REUSEPORT_LB; > - } else if (!IN6_IS_ADDR_UNSPECIFIED(&sin6->sin6_addr)) { > - struct epoch_tracker et; > - struct ifaddr *ifa; > - > - sin6->sin6_port = 0; /* yech... */ > - NET_EPOCH_ENTER(et); > - if ((ifa = ifa_ifwithaddr((struct sockaddr *)sin6)) == > - NULL && > - (inp->inp_flags & INP_BINDANY) == 0) { > - NET_EPOCH_EXIT(et); > - return (EADDRNOTAVAIL); > - } > - > - /* > - * XXX: bind to an anycast address might accidentally > - * cause sending a packet with anycast source address. > - * We should allow to bind to a deprecated address, > since > - * the application dares to use it. > - */ > - if (ifa != NULL && > - ((struct in6_ifaddr *)ifa)->ia6_flags & > - > (IN6_IFF_ANYCAST|IN6_IFF_NOTREADY|IN6_IFF_DETACHED)) { > - NET_EPOCH_EXIT(et); > - return (EADDRNOTAVAIL); > - } > - NET_EPOCH_EXIT(et); > - } > - if (lport) { > - struct inpcb *t; > - > - if (ntohs(lport) <= V_ipport_reservedhigh && > - ntohs(lport) >= V_ipport_reservedlow && > - priv_check_cred(cred, PRIV_NETINET_RESERVEDPORT)) > - return (EACCES); > - > - if (!IN6_IS_ADDR_MULTICAST(&sin6->sin6_addr) && > - priv_check_cred(inp->inp_cred, > PRIV_NETINET_REUSEPORT) != > 0) { > - t = in6_pcblookup_local(pcbinfo, > - &sin6->sin6_addr, lport, > - INPLOOKUP_WILDCARD, cred); > - if (t != NULL && > - (so->so_type != SOCK_STREAM || > - IN6_IS_ADDR_UNSPECIFIED(&t->in6p_faddr)) && > - (!IN6_IS_ADDR_UNSPECIFIED(&sin6->sin6_addr) > || > - !IN6_IS_ADDR_UNSPECIFIED(&t->in6p_laddr) || > - (t->inp_socket->so_options & SO_REUSEPORT) > || > - (t->inp_socket->so_options & > SO_REUSEPORT_LB) == > 0) && > - (inp->inp_cred->cr_uid != > - t->inp_cred->cr_uid)) > - return (EADDRINUSE); > - > -#ifdef INET > - if ((inp->inp_flags & IN6P_IPV6_V6ONLY) == 0 && > - IN6_IS_ADDR_UNSPECIFIED(&sin6->sin6_addr)) { > - struct sockaddr_in sin; > - > - in6_sin6_2_sin(&sin, sin6); > - t = in_pcblookup_local(pcbinfo, > - sin.sin_addr, lport, > - INPLOOKUP_WILDCARD, cred); > - if (t != NULL && > - (so->so_type != SOCK_STREAM || > - in_nullhost(t->inp_faddr)) && > - (inp->inp_cred->cr_uid != > - t->inp_cred->cr_uid)) > - return (EADDRINUSE); > - } > -#endif > - } > - t = in6_pcblookup_local(pcbinfo, &sin6->sin6_addr, > - lport, lookupflags, cred); > - if (t != NULL && ((reuseport | reuseport_lb) & > - t->inp_socket->so_options) == 0) > - return (EADDRINUSE); > -#ifdef INET > - if ((inp->inp_flags & IN6P_IPV6_V6ONLY) == 0 && > - IN6_IS_ADDR_UNSPECIFIED(&sin6->sin6_addr)) { > - struct sockaddr_in sin; > + /* See if this address/port combo is available. */ > + error = in6_pcbbind_avail(inp, sin6, sooptions, lookupflags, > + cred); > + if (error != 0) > + return (error); > > - in6_sin6_2_sin(&sin, sin6); > - t = in_pcblookup_local(pcbinfo, sin.sin_addr, > - lport, lookupflags, cred); > - if (t != NULL && ((reuseport | reuseport_lb) & > - t->inp_socket->so_options) == 0 && > - (!in_nullhost(t->inp_laddr) || > - (t->inp_vflag & INP_IPV6PROTO) != 0)) { > - return (EADDRINUSE); > - } > - } > -#endif > - } > + lport = sin6->sin6_port; > inp->in6p_laddr = sin6->sin6_addr; > } > if (lport == 0) { >
