[PATCH] Core: fixed uninitialized memory access

[Maxim Dounin]

Hello!

On Fri, Sep 30, 2016 at 12:09:12AM +0200, Jan Seda wrote:

> Hello.
> 
> On 2016-09-29, 09:02:16, Maxim Dounin wrote:
> > Thanks for catching this.
> > 
> > Dropping the length returned by getsockname() doesn't look like 
> > a correct solution though.  Instead, teaching ngx_cmp_sockaddr() to 
> > compare sockaddrs with not-completely-filled sun_path - that is, 
> > respecting socklen - should be the right way to go.
> > 
> > Please try the following patch:
> 
> Seems to work OK. Cannot reproduce the problem anymore. Thanks.
> 
> BTW, wouldn't s/ngx_palloc/ngx_pcalloc/ in ngx_set_inherited_sockets()
> be prudent anyway?

It shouldn't be needed unless there is a bug elsewhere.

> Also, I see your patch is prepared for abstract namespace sockets. Is
> this feature planned soon? I cobbled up a patch for that (attached) and
> such sockets now interop with haproxy (abns@ scheme) and socat (with
> unix-tightsocklen=0). But it probably is not production-ready.

No, there are no plans, I've just tried to keep the code more or 
less generic.

Just in case you are interested, below is slightly cleaned up 
version of your patch.

# HG changeset patch
# User Maxim Dounin <mdounin at mdounin.ru>
# Date 1475091257 -7200
#      Wed Sep 28 21:34:17 2016 +0200
# Node ID cd448f5f1eec8d11bd9d942d44aa117de397c9d1
# Parent  9b9ae81cd4f01ed60e7bab323d49b470cec69d9e
Core: support for abstract namespace sockets on Linux.

Abstract sockets recognized in the "unix:@name" form.  Rest of the
struct sockaddr_un's sun_path is filled with zeros, and socklen is set
to be sizeof(struct sockaddr_un).  This is expected to be compatible
with HAProxy (abns@ scheme) and socat (with unix-tightsocklen=0).

Based on a patch by Jan Seda,
http://mailman.nginx.org/pipermail/nginx-devel/2016-September/008851.html.

diff --git a/src/core/ngx_connection.c b/src/core/ngx_connection.c
--- a/src/core/ngx_connection.c
+++ b/src/core/ngx_connection.c
@@ -562,7 +562,9 @@ ngx_open_listening_sockets(ngx_cycle_t *
 
 #if (NGX_HAVE_UNIX_DOMAIN)
 
-            if (ls[i].sockaddr->sa_family == AF_UNIX) {
+            if (ls[i].sockaddr->sa_family == AF_UNIX
+                && ls[i].addr_text.data[sizeof("unix:") - 1] != '@')
+            {
                 mode_t   mode;
                 u_char  *name;
 
@@ -1006,6 +1008,7 @@ ngx_close_listening_sockets(ngx_cycle_t 
 #if (NGX_HAVE_UNIX_DOMAIN)
 
         if (ls[i].sockaddr->sa_family == AF_UNIX
+            && ls[i].addr_text.data[sizeof("unix:") - 1] != '@'
             && ngx_process <= NGX_PROCESS_MASTER
             && ngx_new_binary == 0)
         {
diff --git a/src/core/ngx_cycle.c b/src/core/ngx_cycle.c
--- a/src/core/ngx_cycle.c
+++ b/src/core/ngx_cycle.c
@@ -698,7 +698,9 @@ old_shm_zone_done:
 
 #if (NGX_HAVE_UNIX_DOMAIN)
 
-        if (ls[i].sockaddr->sa_family == AF_UNIX) {
+        if (ls[i].sockaddr->sa_family == AF_UNIX
+            && ls[i].addr_text.data[sizeof("unix:") - 1] != '@')
+        {
             u_char  *name;
 
             name = ls[i].addr_text.data + sizeof("unix:") - 1;
diff --git a/src/core/ngx_inet.c b/src/core/ngx_inet.c
--- a/src/core/ngx_inet.c
+++ b/src/core/ngx_inet.c
@@ -240,6 +240,9 @@ ngx_sock_ntop(struct sockaddr *sa, sockl
         if (socklen <= (socklen_t) offsetof(struct sockaddr_un, sun_path)) {
             p = ngx_snprintf(text, len, "unix:%Z");
 
+        } else if (saun->sun_path[0] == '\0') {
+            p = ngx_snprintf(text, len, "unix:@%s%Z", &saun->sun_path[1]);
+
         } else {
             p = ngx_snprintf(text, len, "unix:%s%Z", saun->sun_path);
         }
@@ -740,6 +743,10 @@ ngx_parse_unix_domain_url(ngx_pool_t *po
     saun->sun_family = AF_UNIX;
     (void) ngx_cpystrn((u_char *) saun->sun_path, path, len);
 
+    if (path[0] == '@') {
+        saun->sun_path[0] = '\0';
+    }
+
     u->addrs = ngx_pcalloc(pool, sizeof(ngx_addr_t));
     if (u->addrs == NULL) {
         return NGX_ERROR;
@@ -756,6 +763,10 @@ ngx_parse_unix_domain_url(ngx_pool_t *po
     saun->sun_family = AF_UNIX;
     (void) ngx_cpystrn((u_char *) saun->sun_path, path, len);
 
+    if (path[0] == '@') {
+        saun->sun_path[0] = '\0';
+    }
+
     u->addrs[0].sockaddr = (struct sockaddr *) saun;
     u->addrs[0].socklen = sizeof(struct sockaddr_un);
     u->addrs[0].name.len = len + 4;

-- 
Maxim Dounin
http://nginx.org/