On Fri, Jun 13, 2014 at 1:09 AM, Yann Ylavic <[email protected]> wrote:
> On Fri, Jun 13, 2014 at 12:32 AM, Yann Ylavic <[email protected]> wrote:
>> The most important imho is to not truncate the length at sizeof(struct
>> sockaddr_un) when the real sun_path is beyond sizeof(sun_path).
>> The libc calls are probably bullet proof regarding NUL termination
>> (eg. force ((char*)sun)[addrlen] = 0 and recompute the length like in
>> the linux code from your link above), setting the NUL ourself at the
>> good place seems reasonable though ;)
>
> Hence I think the correct behaviour is in mod_cgid, and something like
> the following patch should be applied :
This one is better :
Index: modules/proxy/mod_proxy_fdpass.c
===================================================================
--- modules/proxy/mod_proxy_fdpass.c (revision 1602309)
+++ modules/proxy/mod_proxy_fdpass.c (working copy)
@@ -57,7 +57,8 @@ static int proxy_fdpass_canon(request_rec *r, char
/* TODO: In APR 2.x: Extend apr_sockaddr_t to possibly be a path !!! */
/* XXX: The same function exists in proxy_util.c */
static apr_status_t socket_connect_un(apr_socket_t *sock,
- struct sockaddr_un *sa)
+ struct sockaddr_un *sa,
+ apr_socklen_t salen)
{
apr_status_t rv;
apr_os_sock_t rawsock;
@@ -74,9 +75,7 @@ static apr_status_t socket_connect_un(apr_socket_t
}
do {
- const socklen_t addrlen = APR_OFFSETOF(struct sockaddr_un, sun_path)
- + strlen(sa->sun_path) + 1;
- rv = connect(rawsock, (struct sockaddr*)sa, addrlen);
+ rv = connect(rawsock, (struct sockaddr*)sa, salen);
} while (rv == -1 && errno == EINTR);
if ((rv == -1) && (errno == EINPROGRESS || errno == EALREADY)
@@ -103,7 +102,8 @@ static apr_status_t get_socket_from_path(apr_pool_
const char* path,
apr_socket_t **out_sock)
{
- struct sockaddr_un sa;
+ struct sockaddr_un *sa;
+ apr_socklen_t salen, len;
apr_socket_t *s;
apr_status_t rv;
*out_sock = NULL;
@@ -114,10 +114,14 @@ static apr_status_t get_socket_from_path(apr_pool_
return rv;
}
- sa.sun_family = AF_UNIX;
- apr_cpystrn(sa.sun_path, path, sizeof(sa.sun_path));
+ len = strlen(path);
+ salen = APR_OFFSETOF(struct sockaddr_un, sun_path) + len;
+ sa = (struct sockaddr_un *)apr_palloc(p, salen + 1);
+ sa->sun_family = AF_UNIX;
+ memcpy(sa->sun_path, path, len);
+ sa->sun_path[len] = '\0';
- rv = socket_connect_un(s, &sa);
+ rv = socket_connect_un(s, sa, salen);
if (rv != APR_SUCCESS) {
return rv;
}
Index: modules/proxy/proxy_util.c
===================================================================
--- modules/proxy/proxy_util.c (revision 1602309)
+++ modules/proxy/proxy_util.c (working copy)
@@ -2552,7 +2552,8 @@ static apr_status_t send_http_connect(proxy_conn_r
#if APR_HAVE_SYS_UN_H
/* lifted from mod_proxy_fdpass.c; tweaked addrlen in connect() call */
static apr_status_t socket_connect_un(apr_socket_t *sock,
- struct sockaddr_un *sa)
+ struct sockaddr_un *sa,
+ apr_socklen_t salen)
{
apr_status_t rv;
apr_os_sock_t rawsock;
@@ -2569,9 +2570,7 @@ static apr_status_t socket_connect_un(apr_socket_t
}
do {
- const socklen_t addrlen = APR_OFFSETOF(struct sockaddr_un, sun_path)
- + strlen(sa->sun_path) + 1;
- rv = connect(rawsock, (struct sockaddr*)sa, addrlen);
+ rv = connect(rawsock, (struct sockaddr*)sa, salen);
} while (rv == -1 && errno == EINTR);
if ((rv == -1) && (errno == EINPROGRESS || errno == EALREADY)
@@ -2623,7 +2622,8 @@ PROXY_DECLARE(int) ap_proxy_connect_backend(const
#if APR_HAVE_SYS_UN_H
if (conn->uds_path)
{
- struct sockaddr_un sa;
+ struct sockaddr_un *sa;
+ apr_socklen_t salen, len;
rv = apr_socket_create(&newsock, AF_UNIX, SOCK_STREAM, 0,
conn->scpool);
@@ -2638,10 +2638,14 @@ PROXY_DECLARE(int) ap_proxy_connect_backend(const
}
conn->connection = NULL;
- sa.sun_family = AF_UNIX;
- apr_cpystrn(sa.sun_path, conn->uds_path, sizeof(sa.sun_path));
+ len = strlen(conn->uds_path);
+ salen = APR_OFFSETOF(struct sockaddr_un, sun_path) + len;
+ sa = (struct sockaddr_un *)apr_palloc(conn->scpool, salen + 1);
+ sa->sun_family = AF_UNIX;
+ memcpy(sa->sun_path, conn->uds_path, len);
+ sa->sun_path[len] = '\0';
- rv = socket_connect_un(newsock, &sa);
+ rv = socket_connect_un(newsock, sa, salen);
if (rv != APR_SUCCESS) {
apr_socket_close(newsock);
ap_log_error(APLOG_MARK, APLOG_ERR, rv, s, APLOGNO(02454)
[EOS]
