Skip to content
Snippets Groups Projects
New look: Off

crash: double free or corruption (fasttop):

    • Closed (moved) Issue created by toralf @toralf

    stable hardened Gentoo Linux server with 0.3.1.1_alpha2 while reloading config (splitted torrc into smaller files) :

    and again I had to attach the trace isntead just putting it here into b/c TRAC rejected the input as being spam :-/

    To upload designs, you'll need to enable LFS and have an admin enable hashed storage. More information

    Child items ...

    • Activity

    • toralf changed milestone to %Tor: 0.3.1.x-final

      changed milestone to %Tor: 0.3.1.x-final

    • toralf added component::core tor/tor crash milestone::Tor: 0.3.1.x-final owner::Jigsaw52 priority::high regression reload resolution::fixed severity::normal status::closed tor-relay torrc type::defect version::tor 0.3.1.2-alpha labels

      added component::core tor/tor crash milestone::Tor: 0.3.1.x-final owner::Jigsaw52 priority::high regression reload resolution::fixed severity::normal status::closed tor-relay torrc type::defect version::tor 0.3.1.2-alpha labels

    • toralf
      toralf @toralf ·
      Author

      https://paste.pound-python.org/show/wwOXwYuw4erxTJ695rV7/

    • toralf
      toralf @toralf ·
      Author

      Hhm, actually Tor does not crash but spews that if I reload the config unter Gentoo (kill -1 I do assume)

    • toralf
      toralf @toralf ·
      Author

      And it happens, if an %include in torrc points to an non-existing directory and --verify-config is used

    • toralf
      toralf @toralf ·
      Author 
      mr-fox ~ # tor  --verify-config
May 27 16:27:29.867 [notice] Tor 0.3.1.2-alpha (git-61625b8f26384a4a) running on Linux with Libevent 2.1.8-stable, OpenSSL LibreSSL 2.5.4, Zlib 1.2.11, Liblzma 5.2.3, and Libzstd N/A.
May 27 16:27:29.867 [notice] Tor can't help you if you use it wrong! Learn how to be safe at https://www.torproject.org/download/download#warning
May 27 16:27:29.867 [notice] This version is not a stable Tor release. Expect more bugs than usual.
May 27 16:27:29.867 [notice] Read configuration file "/etc/tor/torrc".

============================================================ T= 1495895249
Tor 0.3.1.2-alpha (git-61625b8f26384a4a) died: Caught signal 11
tor(+0x172296)[0x5594c9e62296]
tor(+0x178a1e)[0x5594c9e68a1e]
tor(+0x178a1e)[0x5594c9e68a1e]
tor(config_get_lines_include+0x38)[0x5594c9e68ec8]
tor(options_init_from_string+0x1df)[0x5594c9dedabf]
tor(options_init_from_torrc+0x1f1)[0x5594c9dedfd1]
tor(tor_init+0x313)[0x5594c9d3e1d3]
tor(tor_main+0x6e)[0x5594c9d3f67e]
tor(main+0x28)[0x5594c9d38c98]
/lib64/libc.so.6(__libc_start_main+0xfc)[0x7fc3f452e72c]
tor(_start+0x29)[0x5594c9d38ce9]
Aborted
    • toralf
      toralf @toralf ·
      Author

      FWIW I have /etc/tor/torrc.d, within that afile which contains a single "%include..." line

    • C
      cypherpunks @cypherpunks ·

      Can you attach a minimal torrc which reproduces this issue when passed to tor with -f?

    • toralf
      toralf @toralf ·
      Author

      Trac:
      t.tgz

    • toralf
      toralf @toralf ·
      Author

      tar -cvf t.tgz torrc torrc.d/

    • Nick Mathewson
      Nick Mathewson @nickm ·

      Thanks, Toralf! Does this also happen with 0.3.1.2-alpha? 0.3.1.1-alpha had a known double-free bug.

      Trac:
      Priority: Medium to High
      Keywords: N/A deleted, tor-relay regression torrc reload crash added
      Milestone: N/A to Tor: 0.3.1.x-final

    • toralf
      toralf @toralf ·
      Author

      oh yes, this is 0.3.1.2-alpha - sry for the typo in comment #0

    • C
      cypherpunks @cypherpunks ·

      I'm unable to reproduce the double free. Maybe someone else can try?

    • Roger Dingledine
      Roger Dingledine @arma ·

      I also can't reproduce it, on Debian.

      toralf, can you either give your explicit torrc files (are you running Tor as root like the # prompt implies) or better, run it under valgrind and give us a more useful trace?

      Thanks!

    • J
      Jigsaw52 @Jigsaw52 ·

      I was able to reproduce this and I fixed it. Here is the fix: https://github.com/Jigsaw52/tor/tree/fix-22417

      The bug was triggered by including a folder with a non-empty file without any values followed by another file.

      The problem is in the config_process_include() function. When this function is called to process an included folder, it will call config_get_included_list() for each file in the folder. config_get_included_list() will write the list of values on the file to included_list and the pointer to the last entry of the list to *list_last. When a file contains no values, both included_list and *list_last will be null. The bug is that, after calling config_get_included_list(), we update the the *next pointer with the following code:

          **next = included_list;
    *next = &(*list_last)->next;

      When *list_last is null, we're are updating *next with the offset of next on the config_line_t struct (0x10 on a 64bit system). The next file on the folder will go through the same code and crash when writing to !**next.

      Trac:
      Status: new to needs_review

    • Roger Dingledine
      Roger Dingledine @arma ·

      Replying to Jigsaw52:

      The bug was triggered by including a folder with a non-empty file without any values followed by another file.

      Agreed. For those trying to reproduce it, here's the trick:

      mkdir newdir
echo " " > newdir/a
echo l > newdir/b
echo "%include newdir" > torrc
src/or/tor -f torrc
    • Roger Dingledine
      Roger Dingledine @arma ·

      For those following along, the proposed fix is:

      --- a/src/common/confline.c
+++ b/src/common/confline.c
@@ -300,7 +300,7 @@ config_process_include(const char *path, int recursion_level, int extended,
     tor_free(config_file);
 
     **next = included_list;
-    *next = &(*list_last)->next;
+    *next = !*list_last ? *next : &(*list_last)->next;
 
   } SMARTLIST_FOREACH_END(config_file);
   smartlist_free(config_files);

      I can confirm that the change stops the crash.

      But (not knowing the code here at all, though I admit I am stunned to find a ***next :) it is a confusing fix. If !*list_last, then assign *next to *next? If that's actually what you meant to do, wouldn't it be simpler to do it with something like

          if (*list_last)
      *next = &(*list_last)->next;

      ?

      (I am in a twisty maze of pointers to pointers to pointers, all alike.)

    • toralf
      toralf @toralf ·
      Author

      Replying to cypherpunks:

      I'm unable to reproduce the double free. Maybe someone else can try? Just for completeness:

      The attached t.tgz does contain the non-empty no-vars-containing 60_* file. Shouldn't that help to reproduce the bug ?

    • J
      Jigsaw52 @Jigsaw52 ·

      Assigning *next to *next is a no-op, so the code I wrote does the same as the version with the if. However, the version with the if is indeed easier to read. I've changed it on my branch.

    • C
      cypherpunks @cypherpunks ·

      Replying to toralf:

      Replying to cypherpunks:

      I'm unable to reproduce the double free. Maybe someone else can try? Just for completeness:

      The attached t.tgz does contain the non-empty no-vars-containing 60_* file. Shouldn't that help to reproduce the bug ? I extracted the tarball into a separate directory (not wanting to overwrite global configurations) without changing the %include. This caused Tor to abort prematurely because it couldn't find the /etc/tor/torrc.d/ directory and didn't reproduce the issue. The description in comment:3 lead me down the wrong path so in the end i wasn't able to reproduce it.

      Changing the %include to the torrc.d directory in my extraction directory and the STR from comment:12 both reproduce the issue on my side now.

      I agree with comment:13 that the 3-star (reference [http://wiki.c2.com/?ThreeStarProgrammer]) pointer is less than ideal.

    • J
      Jigsaw52 @Jigsaw52 ·

      Here is a branch without the 3-star pointer: https://github.com/Jigsaw52/tor/tree/fix-22417-without-3-star

      It replaces it with a 2-star pointer but duplicates the code to update the next pointer in config_get_lines_aux.

    • J
      Jigsaw52 @Jigsaw52 ·

      Trac:
      Owner: N/A to Jigsaw52
      Status: needs_review to assigned

    • J
      Jigsaw52 @Jigsaw52 ·

      Trac:
      Status: assigned to needs_review

    • Nick Mathewson
      Nick Mathewson @nickm ·

      merged; added a changes file. Thanks for fixing!

      Trac:
      Status: needs_review to closed
      Resolution: N/A to fixed

    • Trac closed

      closed

    • Trac moved to tpo/core/tor#22417 (closed)

      moved to tpo/core/tor#22417 (closed)

    Please register or sign in to reply