Welcome! Log In Create A New Profile

Advanced

Re: [PATCH] Core: fixed uninitialized memory access

Maxim Dounin
October 04, 2016 05:26PM
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@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/

_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel
Subject Author Views Posted

[PATCH] Core: fixed uninitialized memory access

Jan Seda 491 September 27, 2016 05:24PM

Re: [PATCH] Core: fixed uninitialized memory access

Maxim Dounin 224 September 29, 2016 02:04AM

Re: [PATCH] Core: fixed uninitialized memory access

Jan Seda 206 September 29, 2016 06:10PM

Re: [PATCH] Core: fixed uninitialized memory access

Maxim Dounin 218 October 04, 2016 05:26PM



Sorry, you do not have permission to post/reply in this forum.

Online Users

Guests: 144
Record Number of Users: 8 on April 13, 2023
Record Number of Guests: 421 on December 02, 2018
Powered by nginx      Powered by FreeBSD      PHP Powered      Powered by MariaDB      ipv6 ready