Changeset View
Standalone View
sys/netinet6/sctp6_usrreq.c
Show First 20 Lines • Show All 703 Lines • ▼ Show 20 Lines | if (addr == NULL) { | ||||
SCTP_RELEASE_PKT(m); | SCTP_RELEASE_PKT(m); | ||||
if (control) { | if (control) { | ||||
SCTP_RELEASE_PKT(control); | SCTP_RELEASE_PKT(control); | ||||
control = NULL; | control = NULL; | ||||
} | } | ||||
SCTP_LTRACE_ERR_RET(inp, NULL, NULL, SCTP_FROM_SCTP6_USRREQ, EDESTADDRREQ); | SCTP_LTRACE_ERR_RET(inp, NULL, NULL, SCTP_FROM_SCTP6_USRREQ, EDESTADDRREQ); | ||||
return (EDESTADDRREQ); | return (EDESTADDRREQ); | ||||
} | } | ||||
switch (addr->sa_family) { | |||||
#ifdef INET | |||||
case AF_INET: | |||||
if (addr->sa_len != sizeof(struct sockaddr_in)) { | |||||
SCTP_LTRACE_ERR_RET(inp, NULL, NULL, SCTP_FROM_SCTP6_USRREQ, EINVAL); | |||||
markj: We are leaking mbufs here. Some of the existing checks in this function do as well, as do other… | |||||
return (EINVAL); | |||||
} | |||||
break; | |||||
#endif | |||||
#ifdef INET6 | |||||
case AF_INET6: | |||||
if (addr->sa_len != sizeof(struct sockaddr_in6)) { | |||||
SCTP_LTRACE_ERR_RET(inp, NULL, NULL, SCTP_FROM_SCTP6_USRREQ, EINVAL); | |||||
return (EINVAL); | |||||
} | |||||
break; | |||||
#endif | |||||
default: | |||||
SCTP_LTRACE_ERR_RET(inp, NULL, NULL, SCTP_FROM_SCTP6_USRREQ, EINVAL); | |||||
return (EINVAL); | |||||
} | |||||
#ifdef INET | #ifdef INET | ||||
sin6 = (struct sockaddr_in6 *)addr; | sin6 = (struct sockaddr_in6 *)addr; | ||||
if (SCTP_IPV6_V6ONLY(inp)) { | if (SCTP_IPV6_V6ONLY(inp)) { | ||||
Not Done Inline ActionsI'm just picking this as a good spot to insert a general question/concern/comment. (And, even though your code is a trigger for my comment, my comment is really largely independent of your proposed change and certainly not any sort of criticism of it.) I personally remain a tiny bit confused about when we allow AF_INET/(struct sockaddr_in) on IPv6 sockets. My general understanding is that this is generally a legacy functionality related to IPV4 mapped addresses and should no longer occur. However, our code still seems to include places which allow IPv4 addresses to be used with IPv6 sockets. It seems to me like this change may break that functionality in one or more places, if it is something we want to support. OTOH, if we no longer want to support this, we should probably intentionally clean that up as a separate change to the one which you are proposing. So, can you (or anyone else on this review) give a summary of our position with respect to allowing AF_INET/(struct sockaddr_in) to be used on IPv6 sockets? Related, can anyone summarize our expected support for IPv4 mapped addresses? It is possible that I'm completely confused and my questions are non-sensical. If so, I apologize in advance... jtl: I'm just picking this as a good spot to insert a general question/concern/comment. (And, even… | |||||
Done Inline ActionsI believe that we in fact do not allow AF_INET sockaddrs to be used with AF_INET6 sockets at all. Consider the !SCTP_IPV6_V6ONLY(inp) case here. We check IN6_IS_ADDR_V4MAPPED(&sin6->sin6_addr), i.e., we assume that the sockaddr is a sockaddr_in6. This assumption exists in the TCP and UDP code as well. In all cases we check IN6_IS_ADDR_V4MAPPED() before converting the sockaddr to a sockaddr_in with in6_sin6_2_sin(). markj: I believe that we in fact do not allow AF_INET sockaddrs to be used with AF_INET6 sockets at… | |||||
/* | /* | ||||
* if IPV6_V6ONLY flag, we discard datagrams destined to a | * if IPV6_V6ONLY flag, we discard datagrams destined to a | ||||
* v4 addr or v4-mapped addr | * v4 addr or v4-mapped addr | ||||
*/ | */ | ||||
if (addr->sa_family == AF_INET) { | if (addr->sa_family == AF_INET) { | ||||
SCTP_LTRACE_ERR_RET(inp, NULL, NULL, SCTP_FROM_SCTP6_USRREQ, EINVAL); | SCTP_LTRACE_ERR_RET(inp, NULL, NULL, SCTP_FROM_SCTP6_USRREQ, EINVAL); | ||||
return (EINVAL); | return (EINVAL); | ||||
} | } | ||||
▲ Show 20 Lines • Show All 458 Lines • Show Last 20 Lines |
We are leaking mbufs here. Some of the existing checks in this function do as well, as do other protocols in some cases. I'm working on a separate patch for that.