Le 10/06/2014 18:19, Yann Ylavic a écrit :
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()/
Thx Fixed in r1598946
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?
I didn't take time to do it but it was clearly what I had in mind with
this note.
Should I resubmit?
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...
I didn't look at APR code and missed cgid with my grep.
Just as you, I didn't manage to find some consistent information about it.
-
http://pubs.opengroup.org/onlinepubs/9699919799/functions/connect.html
suggests that the size of the whole structure should be used
(i.e./address_len:///Specifies the length of the *sockaddr* structure
pointed to by the /address/ argument.)
- most examples I found don't add the +1 for the last '\0'
- some examples have this +1 (and this sound logical to me)
- SUN_LEN does not seem to be that used
One of the best post I've found is:
http://stackoverflow.com/questions/2307511/proper-length-of-an-af-unix-socket-when-calling-bind
In mod_proxy_fdpass and in proxy_util, this is safe because sun_path is
filled with apr_cpystrn.
So, leaving the +1 looks safe to me as we will go beyond sizeof(struct
sockaddr_un).
Any one else has an opinion?
CJ
