Repository navigation
null pointer in services/outside_network.c:160 reuse_cmp / rbtree_find_less_equal in Unbound 1.13.0 release #411
Copy link
Copy link
Closed
Description
Using my amd64 linux on centos7 build from #393 (comment) with these commits added to 1.13.0:
I am still getting rare crashes. I've caught one here, in reuse_cmp having a nullptr for key2, coming from node->key:
Lines 525 to 528 in ca49781
| /* While there are children... */ | |
| while (node != RBTREE_NULL) { | |
| r = rbtree->cmp(key, node->key); | |
| if (r == 0) { |
The backtrace is:
(gdb) bt
#0 reuse_cmp_addrportssl (key1=0x7fde2e2737e8, key2=0x0) at services/outside_network.c:144
#1 0x000055e21f6ba7c1 in (key1=0x7fde2e2737e8, key2=0x0) at services/outside_network.c:160
#2 0x000055e21f6759ce in rbtree_find_less_equal (rbtree=rbtree@entry=0x7fdd7a428198, key=key@entry=0x7fde2e2737e8, result=result@entry=0x7fde2e2737c8) at util/rbtree.c:527
#3 0x000055e21f6baf0c in reuse_tcp_find (outnet=outnet@entry=0x7fdd7a428090, addr=addr@entry=0x7fdd6718d6f0, addrlen=16, use_ssl=<optimized out>) at services/outside_network.c:480
#4 0x000055e21f6bbf5f in use_free_buffer (outnet=outnet@entry=0x7fdd7a428090) at services/outside_network.c:723
#5 0x000055e21f6bc4fb in outnet_tcp_cb (c=0x7fdd61fd0ce0, arg=0x7fdd61fd0bb0, error=<optimized out>, reply_info=0x7fdd61fd0d18) at services/outside_network.c:1095
#6 0x000055e21f6b4087 in tcp_callback_reader (c=0x7fdd61fd0ce0) at util/netevent.c:1144
#7 0x000055e21f6b5548 in comm_point_tcp_handle_read (fd=217, c=0x7fdd61fd0ce0, short_ok=0) at util/netevent.c:1668
#8 0x000055e21f6b584b in comm_point_tcp_handle_callback (fd=217, event=<optimized out>, arg=0x7fdd61fd0ce0) at util/netevent.c:2062
#9 0x00007fde30723a14 in event_base_loop () from /lib64/libevent-2.0.so.5
#10 0x000055e21f6b1fac in comm_base_dispatch (b=<optimized out>) at util/netevent.c:246
#11 0x000055e21f62e499 in worker_work (worker=worker@entry=0x55e2216803b0) at daemon/worker.c:1941
#12 0x000055e21f6222bf in thread_start (arg=0x55e2216803b0) at daemon/daemon.c:540
#13 0x00007fde3009bea5 in start_thread () from /lib64/libpthread.so.0
#14 0x00007fde2fdc496d in clone () from /lib64/libc.so.6
At the null pointer, *node is normal except for the null:
(gdb) up
#1 0x000055e21f6ba7c1 in reuse_cmp (key1=0x7fde2e2737e8, key2=0x0) at services/outside_network.c:160
160 r = reuse_cmp_addrportssl(key1, key2);
(gdb) up
#2 0x000055e21f6759ce in rbtree_find_less_equal (rbtree=rbtree@entry=0x7fdd7a428198, key=key@entry=0x7fde2e2737e8, result=result@entry=0x7fde2e2737c8) at util/rbtree.c:527
527 r = rbtree->cmp(key, node->key);
(gdb) p *node
$2 = {parent = 0x7fdd61a09468, left = 0x55e21f922460 <rbtree_null_node>, right = 0x55e21f922460 <rbtree_null_node>, key = 0x0, color = 1 '\001'}
The core file is 6.36 GB; I can certainly share it and the centos7 rpm files out-of-band if you'd like to investigate directly, or I am happy to dig around in the core file in response to your questions. Thanks again!
Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
No labels
Activity
gthess commentedon Jan 27, 2021
Could you provide the configuration in use?
jcjones commentedon Jan 27, 2021
Sure: https://gist.github.com/jcjones/2f7a19b8d0c5b3c4072b5f265f88a4bf
gthess commentedon Jan 27, 2021
Thanks! At least this excludes a path I was looking at: forwarders and tls configuration.
Having a NULL on the key and the node somehow still being part of the tree is the fault here; still looking into it.
If you notice a way to reliably reproduce it that would be a big plus.
jcjones commentedon Jan 27, 2021
I'm afraid a reproducer is not going to be likely given the nature of the input data to these instances; sorry. Still, if something comes up, I'll let you know.
- Attempt to fix NULL keys in the reuse_tcp tree; relates to #411.
gthess commentedon Feb 2, 2021
Hi @jcjones, I wasn't able to reproduce this. We have an attempt on a fix that is included in the ongoing release candidate for unbound 1.13.1. We have a branch for that, if you would like to try it out that would be great.
Other than that, an extra question: do you use
unbound-controlto alter the configuration on the fly?jcjones commentedon Feb 2, 2021
No, we're not using
unbound-control. I'll give the tag a try ASAP, though it may be a few days before I have results. Thanks!jcjones commentedon Feb 9, 2021
Just an update that we've had 24 hours of 1.13.1rc1 so far with only a stuck process (100% CPU, unfortunately didn't get a core on the restart), no segfaults yet.
jcjones commentedon Feb 9, 2021
We've seen a segfault on 1.13.1rc1; I have a core dump I'll analyze tonight.
gthess commentedon Feb 9, 2021
Log output would also be nice if available. We added a couple extra print error cases ("internal error: ...") when trying to fix this.
jcjones commentedon Feb 9, 2021
Merge remote-tracking branch 'nlnet/master'
jcjones commentedon Feb 10, 2021
Stack trace:
I swear I saw internal errors somewhere recently, but perhaps it wasn't unbound. None of my unbound instances have logged "internal error" in accessible logs, nor the specific messages from a8485d5.
Here's the log around the segfault, though it's pretty bare:
jcjones commentedon Mar 4, 2021
I'm still seeing these spuriously with 1.13.1 release, they look the same. E.g.:
No instances of "internal error" in the logs at all. No output from
unboundin the logs anytime close to the segfault.Most interesting to me is that I have two of these same segfaults occurring within 10 minutes of each other, whereas before they always seemed to need substantial time to reproduce:
23 remaining items
- Changelog entry for #513: Stream reuse, attempt to fix #411, #439,
gthess commentedon Jul 26, 2021
Auto-closed by commit message, reopening.
gthess commentedon Jul 26, 2021
Hi @jcjones, @Mityai, @internationils,
Further fixes have been merged (PR #513) to the master branch.
Our own testing does not yield the issue anymore but it would be great if you could test and provide feedback!
rbgarga commentedon Jul 26, 2021
I would love to add this patch on pfSense development branches so more people can test but it's not applying on 1.13.1 cleanly. I'm going to work on fixing conflicts and see if it works
gthess commentedon Jul 26, 2021
Hi @rbgarga,
that would be great!
From a quick look, the conflicts don't appear complicated to me, glad to help if you get stuck. (Make sure to ignore
doc/Changelogas this can give a lot of conflicts).Btw you would also need the following commits (in order, before the PR diff) that solve parts of the issue before the PR was created:
rbgarga commentedon Jul 26, 2021
t
I already have these 3 commits applied and removed test changes for #513 ending up with a patch that only touches services/outside_network.[ch].
2 hunks fail to apply creating this reject file https://idaho.arrakis.com.br/files/outside_network.c.rej
rbgarga commentedon Jul 26, 2021
I sorted out first hunk but second seems to depend of any other change:
gthess commentedon Jul 26, 2021
You need the first part for sure; the else including
outnet_add_tcp_waiting_firstandbreak.This becomes the else to the
if(reuse)above. The previous else has now becomeelse if(outnet->tcp_free).I believe there are no
ifdefs in your current version and you can ignore the second part.Hope this is clear.
rbgarga commentedon Jul 26, 2021
Awesome! Thanks!
Merge remote-tracking branch 'nlnet/master'
gthess commentedon Aug 12, 2021
@rbgarga, 1.13.2 is now released which includes the aforementioned patches. I believe it solves the occasional crash while reloading that I've been reading on the pfsense forum.
@jcjones, @Mityai, we believe 1.13.2 solves this issue. I leave the issue open and feel free to close/update based on your experience.
gthess commentedon Mar 17, 2023
Closing as inactive; the observed issues seem resolved.
Merge pull request NLnetLabs#513 from NLnetLabs/tcp_reuse_fix