Changeset View
Standalone View
sys/netinet/in_pcb.c
Context not available. | |||||
*/ | */ | ||||
#if defined(INET) || defined(INET6) | #if defined(INET) || defined(INET6) | ||||
int | int | ||||
in_pcb_lport(struct inpcb *inp, struct in_addr *laddrp, u_short *lportp, | in_pcb_lport(struct inpcb *inp, struct sockaddr *nam, struct in_addr *laddrp, | ||||
struct ucred *cred, int lookupflags) | u_short *lportp, struct ucred *cred, int lookupflags) | ||||
{ | { | ||||
struct inpcbinfo *pcbinfo; | struct inpcbinfo *pcbinfo; | ||||
struct inpcb *tmpinp; | struct inpcb *tmpinp; | ||||
Context not available. | |||||
u_short aux, first, last, lport; | u_short aux, first, last, lport; | ||||
#ifdef INET | #ifdef INET | ||||
struct in_addr laddr; | struct in_addr laddr; | ||||
struct sockaddr_in *sin = NULL; | |||||
#endif | #endif | ||||
pcbinfo = inp->inp_pcbinfo; | pcbinfo = inp->inp_pcbinfo; | ||||
Context not available. | |||||
KASSERT(laddrp != NULL, ("%s: laddrp NULL for v4 inp %p", | KASSERT(laddrp != NULL, ("%s: laddrp NULL for v4 inp %p", | ||||
__func__, inp)); | __func__, inp)); | ||||
laddr = *laddrp; | laddr = *laddrp; | ||||
sin = (struct sockaddr_in *)nam; | |||||
} | } | ||||
#endif | #endif | ||||
tmpinp = NULL; /* Make compiler happy. */ | tmpinp = NULL; /* Make compiler happy. */ | ||||
Context not available. | |||||
lport = htons(*lastport); | lport = htons(*lastport); | ||||
#ifdef INET6 | #ifdef INET6 | ||||
if ((inp->inp_vflag & INP_IPV6) != 0) | if ((inp->inp_vflag & INP_IPV6) != 0) { | ||||
tmpinp = in6_pcblookup_local(pcbinfo, | struct sockaddr_in6 *sin6 = (struct sockaddr_in6 *)nam; | ||||
julian: can't you do this assign only in the clause that needs it?
| |||||
eriAuthorUnsubmitted Not Done Inline ActionsThere is no other place i can think of where this can be defered. eri: There is no other place i can think of where this can be defered. | |||||
&inp->in6p_laddr, lport, lookupflags, cred); | if (sin6 != NULL && (inp->inp_flags & INP_ANONPORT)) { | ||||
tmpinp = in6_pcblookup_hash_locked(pcbinfo, | |||||
&sin6->sin6_addr, sin6->sin6_port, | |||||
&inp->in6p_laddr, lport, | |||||
lookupflags & (~INPLOOKUP_WILDCARD), | |||||
NULL); | |||||
} else | |||||
tmpinp = in6_pcblookup_local(pcbinfo, | |||||
&inp->in6p_laddr, lport, lookupflags, cred); | |||||
} | |||||
#endif | #endif | ||||
#if defined(INET) && defined(INET6) | #if defined(INET) && defined(INET6) | ||||
else | else | ||||
#endif | #endif | ||||
#ifdef INET | #ifdef INET | ||||
tmpinp = in_pcblookup_local(pcbinfo, laddr, | if (sin != NULL && (inp->inp_flags & INP_ANONPORT)) | ||||
lport, lookupflags, cred); | tmpinp = in_pcblookup_hash_locked(pcbinfo, sin->sin_addr, sin->sin_port, laddr, | ||||
lport, lookupflags & (~INPLOOKUP_WILDCARD), NULL); | |||||
else | |||||
tmpinp = in_pcblookup_local(pcbinfo, laddr, | |||||
Not Done Inline ActionsWhy isn't this being implemented as a modification of the existing logic in in_pcblookup_local()? From a design standpoint, I think distributing the definition of 'is this local port available' to additional places outside of in_pcblookup_local() is a step in the wrong direction. Bypassing in_pcblookup_local() and going straight to a non-wildcard in_pcblookup_hash_locked() lookup here creates a new behavior where an outbound connection can use a local address and port number that is being listened on. Currently, the logic in in_pcblookup_local() prevents such a thing. I am not sure if this is the 'collision' between a listening socket and the socket being allocated that julian@ was referring to, but certainly it is a change in behavior that is not necessary for the concept of this patch and one that I think should not be readily accepted as a side effect, and possibly is a change that is undesirable. pkelsey: Why isn't this being implemented as a modification of the existing logic in in_pcblookup_local… | |||||
Not Done Inline ActionsIt has not been done in in_pcb_lport since it means removing completely the logic on random port selection there which is used on anonymous binds. eri: It has not been done in in_pcb_lport since it means removing completely the logic on random… | |||||
lport, lookupflags, cred); | |||||
Not Done Inline Actionsstyle(9): Line too long garga: style(9): Line too long | |||||
#endif | #endif | ||||
} while (tmpinp != NULL); | } while (tmpinp != NULL); | ||||
Context not available. | |||||
return (EINVAL); | return (EINVAL); | ||||
if ((so->so_options & (SO_REUSEADDR|SO_REUSEPORT)) == 0) | if ((so->so_options & (SO_REUSEADDR|SO_REUSEPORT)) == 0) | ||||
lookupflags = INPLOOKUP_WILDCARD; | lookupflags = INPLOOKUP_WILDCARD; | ||||
if (nam == NULL) { | if (nam == NULL || ((*lportp) == 0 && (inp->inp_flags & INP_ANONPORT))) { | ||||
if ((error = prison_local_ip4(cred, &laddr)) != 0) | if ((error = prison_local_ip4(cred, &laddr)) != 0) | ||||
return (error); | return (error); | ||||
} else { | } else { | ||||
Not Done Inline Actionsstyle(9): Line too long garga: style(9): Line too long | |||||
Context not available. | |||||
if (*lportp != 0) | if (*lportp != 0) | ||||
lport = *lportp; | lport = *lportp; | ||||
if (lport == 0) { | if (lport == 0) { | ||||
error = in_pcb_lport(inp, &laddr, &lport, cred, lookupflags); | error = in_pcb_lport(inp, nam, &laddr, &lport, cred, lookupflags); | ||||
if (error != 0) | if (error != 0) | ||||
return (error); | return (error); | ||||
Context not available. | |||||
Not Done Inline Actionsstyle(9): Line too long garga: style(9): Line too long |
can't you do this assign only in the clause that needs it?