Fix regression in in_pcbbind(), bug observable with INET6

classic Classic list List threaded Threaded
2 messages Options
Reply | Threaded
Open this post in threaded view
|

Fix regression in in_pcbbind(), bug observable with INET6

Vincent Gross-5
The regression test in regress/sys/netinet6/autoport is failing because
my merge of in_pcbbind() and in6_pcbbind() introduced a bug. Long story
short, if nam == NULL, then you skip the part where you check if the socket
is already bound based on inp_laddr/inp_laddr6. Also INPLOOKUP_IPV6 is not
set when dealing with INET6 sockets.

The diff below fixes that by moving the 'if (nam){}' inside the
'switch (sotopf(so)){}'.

Ok ?

Index: in_pcb.c
===================================================================
RCS file: /cvs/src/sys/netinet/in_pcb.c,v
retrieving revision 1.200
diff -u -p -r1.200 in_pcb.c
--- in_pcb.c    5 Apr 2016 21:21:41 -0000       1.200
+++ in_pcb.c    6 Apr 2016 15:03:50 -0000
@@ -297,35 +297,36 @@ in_pcbbind(struct inpcb *inp, struct mbu
             (so->so_options & SO_ACCEPTCONN) == 0))
                wild = INPLOOKUP_WILDCARD;
 
-       if (nam) {
-               switch (sotopf(so)) {
+       switch (sotopf(so)) {
 #ifdef INET6
-               case PF_INET6: {
-                       struct sockaddr_in6 *sin6;
-                       if (TAILQ_EMPTY(&in6_ifaddr))
-                               return (EADDRNOTAVAIL);
-                       if (!IN6_IS_ADDR_UNSPECIFIED(&inp->inp_laddr6))
-                               return (EINVAL);
+       case PF_INET6:
+               if (TAILQ_EMPTY(&in6_ifaddr))
+                       return (EADDRNOTAVAIL);
+               if (!IN6_IS_ADDR_UNSPECIFIED(&inp->inp_laddr6))
+                       return (EINVAL);
+               wild |= INPLOOKUP_IPV6;
 
+               if (nam) {
+                       struct sockaddr_in6 *sin6;
                        sin6 = mtod(nam, struct sockaddr_in6 *);
                        if (nam->m_len != sizeof(struct sockaddr_in6))
                                return (EINVAL);
                        if (sin6->sin6_family != AF_INET6)
                                return (EAFNOSUPPORT);
 
-                       wild |= INPLOOKUP_IPV6;
                        if ((error = in6_pcbaddrisavail(inp, sin6, wild, p)))
                                return (error);
                        laddr = &sin6->sin6_addr;
                        lport = sin6->sin6_port;
-                       break;
                }
+               break;
 #endif
-               case PF_INET: {
-                       struct sockaddr_in *sin;
-                       if (inp->inp_laddr.s_addr != INADDR_ANY)
-                               return (EINVAL);
+       case PF_INET:
+               if (inp->inp_laddr.s_addr != INADDR_ANY)
+                       return (EINVAL);
 
+               if (nam) {
+                       struct sockaddr_in *sin;
                        sin = mtod(nam, struct sockaddr_in *);
                        if (nam->m_len != sizeof(*sin))
                                return (EINVAL);
@@ -336,11 +337,10 @@ in_pcbbind(struct inpcb *inp, struct mbu
                                return (error);
                        laddr = &sin->sin_addr;
                        lport = sin->sin_port;
-                       break;
-               }
-               default:
-                       return (EINVAL);
                }
+               break;
+       default:
+               return (EINVAL);
        }
 
        if (lport == 0) {

Reply | Threaded
Open this post in threaded view
|

Re: Fix regression in in_pcbbind(), bug observable with INET6

Alexander Bluhm
On Wed, Apr 06, 2016 at 05:20:32PM +0200, Vincent Gross wrote:

> The regression test in regress/sys/netinet6/autoport is failing because
> my merge of in_pcbbind() and in6_pcbbind() introduced a bug. Long story
> short, if nam == NULL, then you skip the part where you check if the socket
> is already bound based on inp_laddr/inp_laddr6. Also INPLOOKUP_IPV6 is not
> set when dealing with INET6 sockets.
>
> The diff below fixes that by moving the 'if (nam){}' inside the
> 'switch (sotopf(so)){}'.
>
> Ok ?

OK bluhm@

>
> Index: in_pcb.c
> ===================================================================
> RCS file: /cvs/src/sys/netinet/in_pcb.c,v
> retrieving revision 1.200
> diff -u -p -r1.200 in_pcb.c
> --- in_pcb.c    5 Apr 2016 21:21:41 -0000       1.200
> +++ in_pcb.c    6 Apr 2016 15:03:50 -0000
> @@ -297,35 +297,36 @@ in_pcbbind(struct inpcb *inp, struct mbu
>              (so->so_options & SO_ACCEPTCONN) == 0))
>                 wild = INPLOOKUP_WILDCARD;
>  
> -       if (nam) {
> -               switch (sotopf(so)) {
> +       switch (sotopf(so)) {
>  #ifdef INET6
> -               case PF_INET6: {
> -                       struct sockaddr_in6 *sin6;
> -                       if (TAILQ_EMPTY(&in6_ifaddr))
> -                               return (EADDRNOTAVAIL);
> -                       if (!IN6_IS_ADDR_UNSPECIFIED(&inp->inp_laddr6))
> -                               return (EINVAL);
> +       case PF_INET6:
> +               if (TAILQ_EMPTY(&in6_ifaddr))
> +                       return (EADDRNOTAVAIL);
> +               if (!IN6_IS_ADDR_UNSPECIFIED(&inp->inp_laddr6))
> +                       return (EINVAL);
> +               wild |= INPLOOKUP_IPV6;
>  
> +               if (nam) {
> +                       struct sockaddr_in6 *sin6;
>                         sin6 = mtod(nam, struct sockaddr_in6 *);
>                         if (nam->m_len != sizeof(struct sockaddr_in6))
>                                 return (EINVAL);
>                         if (sin6->sin6_family != AF_INET6)
>                                 return (EAFNOSUPPORT);
>  
> -                       wild |= INPLOOKUP_IPV6;
>                         if ((error = in6_pcbaddrisavail(inp, sin6, wild, p)))
> return (error);
>                         laddr = &sin6->sin6_addr;
>                         lport = sin6->sin6_port;
> -                       break;
>                 }
> +               break;
>  #endif
> -               case PF_INET: {
> -                       struct sockaddr_in *sin;
> -                       if (inp->inp_laddr.s_addr != INADDR_ANY)
> -                               return (EINVAL);
> +       case PF_INET:
> +               if (inp->inp_laddr.s_addr != INADDR_ANY)
> +                       return (EINVAL);
>  
> +               if (nam) {
> +                       struct sockaddr_in *sin;
>                         sin = mtod(nam, struct sockaddr_in *);
>                         if (nam->m_len != sizeof(*sin))
>                                 return (EINVAL);
> @@ -336,11 +337,10 @@ in_pcbbind(struct inpcb *inp, struct mbu
>                                 return (error);
>                         laddr = &sin->sin_addr;
>                         lport = sin->sin_port;
> -                       break;
> -               }
> -               default:
> -                       return (EINVAL);
>                 }
> +               break;
> +       default:
> +               return (EINVAL);
>         }
>  
>         if (lport == 0) {