Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

segfault in v3.4 #346

Closed
msaf1980 opened this issue Oct 30, 2018 · 8 comments
Closed

segfault in v3.4 #346

msaf1980 opened this issue Oct 30, 2018 · 8 comments
Labels

Comments

@msaf1980
Copy link
Contributor

msaf1980 commented Oct 30, 2018

Hello.

I get segfault for version 3.4. Synthetic test with UDP packets don't reproduce crash. May be problem near dispatcher.c:615 ? And istead ofcode block from 621 line executed code from line 617

[5435108.569811] carbon-c-relay[7083]: segfault at 7fa43d80d8d1 ip 00005589bfd469e0 sp 00007fa43affecc0 error 6 in carbon-c-relay[5589bfd3f000+25000]

(gdb) bt
#0 0x0000558da623e9e0 in udpsockread (strm=0x558da75d1400, buf=, sze=) at dispatcher.c:162
#1 0x0000558da623fcf3 in dispatch_connection (start=..., self=0x558da75b8c00, conn=0x7fa05dddb8c0) at dispatcher.c:813
#2 dispatch_runner (arg=) at dispatcher.c:1091
#3 0x00007fa0d8067e25 in pthread_mutex_timedlock () from /lib64/libpthread.so.0
#4 0x0000000000000000 in ?? ()

(gdb) list
157 ret = recvfrom(s->sock, buf, sze, 0, (struct sockaddr )&s->saddr, &slen);
158 if (ret <= 0)
159 return ret;
160
161 /
figure out who's calling */
162 s->srcaddr[0] = '\0';
163 switch (s->saddr.sin6_family) {
164 case PF_INET:
165 inet_ntop(s->saddr.sin6_family,
166 &((struct sockaddr_in *)&s->saddr)->sin_addr,

(gdb) p_v strm
"strm" = (z_strm ) <0x558da75d1400> { ptr = {
"strmread" = (ssize_t (
)(struct _z_strm *, void , size_t)) <0x558da623e9a0>,
"strmclose" = (int (
)(struct _z_strm *)) <0x558da623e940>,
"hdl" = (union {...}) <0x558da75d1410> {
"sock" = (int) 10,
"udp" =
(struct udp_strm) <0x558da75d1410> {sock = 10, saddr = {sin6_family = 2, sin6_port = 51495, sin6_flowinfo = 3066564910, sin6_addr = {__in6_u = {__u6_addr8 = '\000' <repeats 15 times>, __u6_addr16 = {0, 0, 0, 0, 0, 0, 0, 0}, __u6_addr32 = {0, 0, 0, 0}}}, sin6_scope_id = 0},
srcaddr = 0x7fa0ab6b58d1 <error: Cannot access memory at address 0x7fa0ab6b58d1>,
srcaddrlen = 24
},
},
"ibuf" = (char [8192]) <0x558da75d1440> { str_len:0 [0:399] = "" + \0 },
"ipos" = (short unsigned int) 0,
"nextstrm" = (struct _z_strm *) <0x0>,
} }

@grobian
Copy link
Owner

grobian commented Oct 30, 2018

Hi, I agree that this doesn't look good. Can you share the server configuration for this particular instance?

@msaf1980
Copy link
Contributor Author

msaf1980 commented Oct 30, 2018

carbon-c-relay started with parameters
"-f /etc/carbon-c-relay.conf -p 2003 -w 8 -q 15000000 -l /var/log/carbon-c-relay/carbon-c-relay.log"

Metric sended over UDP and TCP

Configuration file

cluster alert
any_of alert-1:2003 alert-2:2003 alert-3:2003 ;
cluster clickhouse
any_of clickhouse-1:2003 clickhouse-2:2003 clickhouse-3:2003;

match *
validate ^[0-9.eE+-]+\ [0-9]{10}$ else drop
;

match [0-9A-Fa-f]{8}-?[0-9A-Fa-f]{4}-?[0-9A-Fa-f]{4}-?[0-9A-Fa-f]{4}-?[0-9A-Fa-f]{12}
send to blackhole
stop
;

match *
send to alert clickhouse;

@grobian grobian added the bug label Oct 30, 2018
@grobian
Copy link
Owner

grobian commented Oct 30, 2018

Ah, so it shouldn't do udp at all, am I correct?

@msaf1980
Copy link
Contributor Author

Some metric sended over UDP. More metric sended over UDP.

I check source code and found one place that may be a problem source.

It's a allocation or assignment for connections[c].strm->hdl.udp.srcaddr

One place with this code started from dispatcher.c:615. May be error on lsnr check ?
But I can't reproduce this on test instance.

    connections[c].strm->nextstrm = NULL;
    if (lsnr == NULL || (lsnr->transport & ~0xFFFF) != W_SSL) {
            if (lsnr == NULL || lsnr->ctype != CON_UDP) {
                    connections[c].strm->hdl.sock = sock;

..
} else {
connections[c].strm->hdl.udp.sock = sock;
connections[c].strm->hdl.udp.srcaddr =
connections[c].srcaddr;
connections[c].strm->hdl.udp.srcaddrlen =
sizeof(connections[c].srcaddr);
..
}

@grobian
Copy link
Owner

grobian commented Oct 30, 2018

Yes, you're right. So you have a UDP listener setup, is that correct?

@msaf1980
Copy link
Contributor Author

No listener configured, so

If no listen directive is present, the relay will use the default listeners for port 2003 on tcp and udp, plus the unix socket /tmp/.s.carbon-c-relay.2003

@msaf1980
Copy link
Contributor Author

msaf1980 commented Mar 25, 2019

Trace with ASAN
==11648==ERROR: AddressSanitizer: heap-use-after-free on address 0x7f16b52e9811 at pc 0x00000040f9d6 bp 0x7f16b3ae39d0 sp 0x7f16b3ae39c0
WRITE of size 1 at 0x7f16b52e9811 thread T4
#0 0x40f9d5 in udpsockread /data/git/carbon-c-relay/dispatcher.c:163
#1 0x413154 in dispatch_connection /data/git/carbon-c-relay/dispatcher.c:859
#2 0x4152c4 in dispatch_runner /data/git/carbon-c-relay/dispatcher.c:1137
#3 0x7f16bcca8593 in start_thread (/lib64/libpthread.so.0+0x7593)
#4 0x7f16bc9dbe6e in clone (/lib64/libc.so.6+0xf9e6e)

0x7f16b52e9811 is located 17 bytes inside of 68247552-byte region [0x7f16b52e9800,0x7f16b93ff800)
freed by thread T1 here:
#0 0x7f16be05f448 in __interceptor_realloc (/lib64/libasan.so.5+0xef448)
#1 0x411126 in dispatch_addconnection /data/git/carbon-c-relay/dispatcher.c:600
#2 0x414dd0 in dispatch_runner /data/git/carbon-c-relay/dispatcher.c:1100
#3 0x7f16bcca8593 in start_thread (/lib64/libpthread.so.0+0x7593)

Realloc of connections is a root cause of segfault
https://github.com/grobian/carbon-c-relay/blob/master/dispatcher.c#L600

I see 2 ways for fix this:

  1. Lazy reinit udp struct before read from socket
    msaf1980@7abe979
  2. Reinit udp structuras after realloc. Possibly more correct variant
    msaf1980@c23b85e

grobian pushed a commit that referenced this issue Mar 25, 2019
…alloc

Fixes issue #346.

Signed-off-by: Fabian Groffen <grobian@gentoo.org>
@grobian
Copy link
Owner

grobian commented Mar 25, 2019

Thanks, the realloc indeed is the problem, I applied your patch!

@grobian grobian closed this as completed Mar 25, 2019
grobian added a commit that referenced this issue Mar 25, 2019
Signed-off-by: Fabian Groffen <grobian@gentoo.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants