| Message ID | 50cca375d8730b5bf74b975d0fede64b1a3744c4.1652368648.git.asml.silence@gmail.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | UDP/IPv6 refactoring | expand |
On Fri, 2022-05-13 at 16:26 +0100, Pavel Begunkov wrote: > For AF_INET6 sockets we care the most about ipv6 but not ip4 mappings as > it's requires some extra hops anyway. Take AF_INET6 case from the address > parsing switch and add an explicit path for it. It removes some extra > ifs from the path and removes the switch overhead. > > Signed-off-by: Pavel Begunkov <asml.silence@gmail.com> > --- > net/ipv6/udp.c | 37 +++++++++++++++++-------------------- > 1 file changed, 17 insertions(+), 20 deletions(-) > > diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c > index 85bff1252f5c..e0b1bea998ce 100644 > --- a/net/ipv6/udp.c > +++ b/net/ipv6/udp.c > @@ -1360,30 +1360,27 @@ int udpv6_sendmsg(struct sock *sk, struct msghdr *msg, size_t len) > > /* destination address check */ > if (sin6) { > - if (addr_len < offsetof(struct sockaddr, sa_data)) > - return -EINVAL; > + if (addr_len < SIN6_LEN_RFC2133 || sin6->sin6_family != AF_INET6) { > + if (addr_len < offsetof(struct sockaddr, sa_data)) > + return -EINVAL; I think you can't access 'sin6->sin6_family' before validating the socket address len, that is before doing: if (addr_len < offsetof(struct sockaddr, sa_data)) Paolo
On 5/16/22 14:14, Paolo Abeni wrote: > On Fri, 2022-05-13 at 16:26 +0100, Pavel Begunkov wrote: >> For AF_INET6 sockets we care the most about ipv6 but not ip4 mappings as >> it's requires some extra hops anyway. Take AF_INET6 case from the address >> parsing switch and add an explicit path for it. It removes some extra >> ifs from the path and removes the switch overhead. >> >> Signed-off-by: Pavel Begunkov <asml.silence@gmail.com> >> --- >> net/ipv6/udp.c | 37 +++++++++++++++++-------------------- >> 1 file changed, 17 insertions(+), 20 deletions(-) >> >> diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c >> index 85bff1252f5c..e0b1bea998ce 100644 >> --- a/net/ipv6/udp.c >> +++ b/net/ipv6/udp.c >> @@ -1360,30 +1360,27 @@ int udpv6_sendmsg(struct sock *sk, struct msghdr *msg, size_t len) >> >> /* destination address check */ >> if (sin6) { >> - if (addr_len < offsetof(struct sockaddr, sa_data)) >> - return -EINVAL; >> + if (addr_len < SIN6_LEN_RFC2133 || sin6->sin6_family != AF_INET6) { >> + if (addr_len < offsetof(struct sockaddr, sa_data)) >> + return -EINVAL; > > I think you can't access 'sin6->sin6_family' before validating the > socket address len, that is before doing: Paolo, thanks for reviewing it! sin6_family is protected by if (addr_len < SIN6_LEN_RFC2133 ...) on the previous line. I can add a BUILD_BUG_ON() if that would be more reassuring. > > if (addr_len < offsetof(struct sockaddr, sa_data))
diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c index 85bff1252f5c..e0b1bea998ce 100644 --- a/net/ipv6/udp.c +++ b/net/ipv6/udp.c @@ -1360,30 +1360,27 @@ int udpv6_sendmsg(struct sock *sk, struct msghdr *msg, size_t len) /* destination address check */ if (sin6) { - if (addr_len < offsetof(struct sockaddr, sa_data)) - return -EINVAL; + if (addr_len < SIN6_LEN_RFC2133 || sin6->sin6_family != AF_INET6) { + if (addr_len < offsetof(struct sockaddr, sa_data)) + return -EINVAL; - switch (sin6->sin6_family) { - case AF_INET6: - if (addr_len < SIN6_LEN_RFC2133) + switch (sin6->sin6_family) { + case AF_INET: + goto do_udp_sendmsg; + case AF_UNSPEC: + msg->msg_name = sin6 = NULL; + msg->msg_namelen = addr_len = 0; + goto no_daddr; + default: return -EINVAL; - daddr = &sin6->sin6_addr; - if (ipv6_addr_any(daddr) && - ipv6_addr_v4mapped(&np->saddr)) - ipv6_addr_set_v4mapped(htonl(INADDR_LOOPBACK), - daddr); - break; - case AF_INET: - goto do_udp_sendmsg; - case AF_UNSPEC: - msg->msg_name = sin6 = NULL; - msg->msg_namelen = addr_len = 0; - daddr = NULL; - break; - default: - return -EINVAL; + } } + + daddr = &sin6->sin6_addr; + if (ipv6_addr_any(daddr) && ipv6_addr_v4mapped(&np->saddr)) + ipv6_addr_set_v4mapped(htonl(INADDR_LOOPBACK), daddr); } else { +no_daddr: if (sk->sk_state != TCP_ESTABLISHED) return -EDESTADDRREQ; daddr = &sk->sk_v6_daddr;
For AF_INET6 sockets we care the most about ipv6 but not ip4 mappings as it's requires some extra hops anyway. Take AF_INET6 case from the address parsing switch and add an explicit path for it. It removes some extra ifs from the path and removes the switch overhead. Signed-off-by: Pavel Begunkov <asml.silence@gmail.com> --- net/ipv6/udp.c | 37 +++++++++++++++++-------------------- 1 file changed, 17 insertions(+), 20 deletions(-)