On Sun, Jun 1, 2014 at 8:54 AM,  <[email protected]> wrote:
> Author: jailletc36
> Date: Sun Jun  1 06:54:15 2014
> New Revision: 1598946
>
> URL: http://svn.apache.org/r1598946
> Log:
> Fix computation of the size of 'struct sockaddr_un' when passed to 'connec()'.

s/connec()/connect()/

> Use the same logic as the one in ' in 'proxy_util.c'.
>
> Modified:
>     httpd/httpd/trunk/CHANGES
>     httpd/httpd/trunk/modules/proxy/mod_proxy_fdpass.c
>
[...]
> Modified: httpd/httpd/trunk/modules/proxy/mod_proxy_fdpass.c
> URL: 
> http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/mod_proxy_fdpass.c?rev=1598946&r1=1598945&r2=1598946&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/modules/proxy/mod_proxy_fdpass.c (original)
> +++ httpd/httpd/trunk/modules/proxy/mod_proxy_fdpass.c Sun Jun  1 06:54:15 
> 2014
> @@ -55,6 +55,7 @@ static int proxy_fdpass_canon(request_re
>  }
>
>  /* TODO: In APR 2.x: Extend apr_sockaddr_t to possibly be a path !!! */
> +/* XXX: The same function exists in proxy_util.c */

Shouldn't there be ap_proxy_socket_connect_un() then?

>  static apr_status_t socket_connect_un(apr_socket_t *sock,
>                                        struct sockaddr_un *sa)
>  {
> @@ -73,8 +74,9 @@ static apr_status_t socket_connect_un(ap
>      }
>
>      do {
> -        rv = connect(rawsock, (struct sockaddr*)sa,
> -                               sizeof(*sa) + strlen(sa->sun_path));
> +        const socklen_t addrlen = APR_OFFSETOF(struct sockaddr_un, sun_path)
> +                                  + strlen(sa->sun_path) + 1;

Do we need +1 here?

It seems that cgid_init() and apr_generate_random_bytes() (when
HAVE_EGD) don't, whereas apr_sockaddr_info_get() and
apr_sockaddr_vars_set() use (the faulty) sizeof(struct sockaddr_un).

Most examples I have found don't include the '\0' (so isn't the
SUN_LEN macro, which maybe you should use here).
According to man UNIX(7) though, the +1 seems to be included by
getsockname(), getpeername(), and accept() in their *addrlen output.

Looks like connect() is not trusting the given length, and finds the
'\0' by itself...

> +        rv = connect(rawsock, (struct sockaddr*)sa, addrlen);
>      } while (rv == -1 && errno == EINTR);
>
>      if ((rv == -1) && (errno == EINPROGRESS || errno == EALREADY)
>
>

Reply via email to