Struct sockaddr was allocated without zeroing. As getsockname() does not
necessarily fill all the bytes, this left potentially uninitialized
memory, which caused me a weird failure in the on-the-fly upgrade
mechanism, where the new master failed to pair the config file AF_UNIX
sockets with the inherited ones. ngx_cmp_sockaddr() would return
NGX_DECLINED because the inherited one would contain uninitialized
garbage after the actual name. This in turn would cause nginx to try to
bind() the unix socket again, and obviously fail, because the listening
socket was bound already (and inherited).

Easy to reproduce on debian wheezy, debian jessie and CentOS 7 systems
(all x86_64) with statically linked openssl 1.1.0. The attached patch
seems to fix the problem.

Also harmonized socklen calculation for AF_UNIX sockets to the way
ngx_parse_unix_domain_url() does it (sizeof(struct sockaddr_un)), so
ngx_cmp_sockaddr() is now less dangerous.

Applies cleanly to 1.11.4.


commit 6f2a907b0dc870314e1d1ecc24fd59c60553152e
Author: Jan Seda <hodor@hodor.cz>
Date: Tue Sep 27 21:52:06 2016 +0200

Core: fixed uninitialized memory access

Struct sockaddr was allocated without zeroing.

Also harmonized socklen calculation for AF_UNIX sockets.

diff --git a/src/core/ngx_connection.c b/src/core/ngx_connection.c
index 16ba630..fe6e64f 100644
--- a/src/core/ngx_connection.c
+++ b/src/core/ngx_connection.c
@@ -151,7 +151,7 @@ ngx_set_inherited_sockets(ngx_cycle_t *cycle)
ls = cycle->listening.elts;
for (i = 0; i < cycle->listening.nelts; i++) {

- ls[i].sockaddr = ngx_palloc(cycle->pool, sizeof(ngx_sockaddr_t));
+ ls[i].sockaddr = ngx_pcalloc(cycle->pool, sizeof(ngx_sockaddr_t));
if (ls[i].sockaddr == NULL) {
return NGX_ERROR;
}
@@ -178,6 +178,7 @@ ngx_set_inherited_sockets(ngx_cycle_t *cycle)
case AF_UNIX:
ls[i].addr_text_max_len = NGX_UNIX_ADDRSTRLEN;
len = NGX_UNIX_ADDRSTRLEN;
+ ls[i].socklen = sizeof(struct sockaddr_un);
break;
#endif

@@ -1323,7 +1324,7 @@ ngx_connection_local_sockaddr(ngx_connection_t *c, ngx_str_t *s,
return NGX_ERROR;
}

- c->local_sockaddr = ngx_palloc(c->pool, len);
+ c->local_sockaddr = ngx_pcalloc(c->pool, sizeof(ngx_sockaddr_t));
if (c->local_sockaddr == NULL) {
return NGX_ERROR;
}
@@ -1331,6 +1332,11 @@ ngx_connection_local_sockaddr(ngx_connection_t *c, ngx_str_t *s,
ngx_memcpy(c->local_sockaddr, &sa, len);

c->local_socklen = len;
+#if (NGX_HAVE_UNIX_DOMAIN)
+ if(sa.sockaddr.sa_family == AF_UNIX) {
+ c->local_socklen = sizeof(struct sockaddr_un);
+ }
+#endif
}

if (s == NULL) {


--
Jan Seda
_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel