summaryrefslogtreecommitdiffstats
path: root/src
diff options
context:
space:
mode:
authorSergei Trofimovich <slyfox@gentoo.org>2020-05-01 21:55:05 +0200
committerSergei Trofimovich <slyfox@gentoo.org>2020-05-01 22:39:44 +0200
commit7526e9fe68c324599f0babb65915522d2a9a7dd7 (patch)
treea55ab06fa0e0e83316afae6930a40ed092879f31 /src
parentMerge pull request #24 from Polynomial-C/enable-threads_build_fix (diff)
downloadhaveged-7526e9fe68c324599f0babb65915522d2a9a7dd7.tar.xz
haveged-7526e9fe68c324599f0babb65915522d2a9a7dd7.zip
haveged: fix crash on shutdown in threaded mode
The bug happens when havege is built with --enable-threads. On shutdown the following crash happens: ``` 3109 p = mem2chunk (mem); (gdb) bt #0 __GI___libc_free (mem=0x7f630c066000) at malloc.c:3109 #1 0x00007f630c035f9e in havege_destroy (hptr=0x564b56b6b900) at havege.c:197 #2 0x0000564b561c3dca in error_exit (format=<optimized out>) at haveged.c:708 #3 0x0000564b561c33f1 in run_daemon (argv=0x7ffce7627758, path=0x564b56b6b2a0 ``` valgrind helped me to understand it was a mmap() / free() mismatch: ``` For lists of detected and suppressed errors, rerun with: -s ERROR SUMMARY: 14 errors from 2 contexts (suppressed: 0 from 0) Invalid free() / delete / delete[] / realloc() at 0x48389CB: free (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so) by 0x48AED9F: havege_destroy (havege.c:197) by 0x10BCA5: error_exit (haveged.c:708) by 0x10B1B7: run_daemon (haveged.c:573) by 0x10B1B7: main (haveged.c:470) Address 0x4845000 is in a rw- anonymous segment ``` It happens due to a few factors: 1. havege is built in threaded mode, there parent pid is stored in shared memory and is expected to free resources. 2. havege is ran in daemon mode, that means havege changes pid when detaches from terminal with daemon(). Combination of [1.] and [2.] causes main process to avoid munmap() and inctead fallback to free() at: ```c void havege_destroy(H_PTR hptr) { ... if (!havege_exit(hptr)) // <- here incorrect pid-based detection happens return; if (0 != (temp=hptr->io_buf)) { hptr->io_buf = 0; free(temp); // <--- here free() happens } ``` The change adds a helper to update parent pid with `havege_reparent` helper. It might not be a very clean fix, but it should be good enough to illustrate the problem. Bug: https://bugs.gentoo.org/720286 Reported-by: Marcin Mirosław Signed-off-by: Sergei Trofimovich <slyfox@gentoo.org>
Diffstat (limited to 'src')
-rw-r--r--src/havege.c13
-rw-r--r--src/havege.h8
-rw-r--r--src/haveged.c4
3 files changed, 24 insertions, 1 deletions
diff --git a/src/havege.c b/src/havege.c
index 3aeaf20..6710458 100644
--- a/src/havege.c
+++ b/src/havege.c
@@ -178,6 +178,19 @@ H_PTR havege_create( /* RETURN: app state */
havege_ndsetup(h);
return h;
}
+
+void havege_reparent(
+ H_PTR hptr)
+{
+#if NUMBER_CORES>1
+ H_THREAD *t = (H_THREAD *)hptr->threads;
+ if (0 == t)
+ return; /* single-threaded */
+
+ t->main = getpid();
+#endif
+}
+
/**
* Destructor. In a multi-collector build, this method should be called from a signal handler
* to avoid creating processes.
diff --git a/src/havege.h b/src/havege.h
index 2e1ade6..35bda94 100644
--- a/src/havege.h
+++ b/src/havege.h
@@ -238,6 +238,14 @@ typedef enum {
* H_NOINIT
*/
H_PTR havege_create(H_PARAMS *params);
+
+/**
+ * haveger_create() remembers parent pid and uses it to identify deallocating thread.
+ * daemonize() forks parent and effectively loses parent thread.
+ * havege_reparent(void) allows recovering new parent pid before havege_run() is started.
+ */
+void havege_reparent(H_PTR hptr);
+
/**
* Frees all allocated anchor resources. If the multi-core option is used, this
* method should be called from a signal handler to prevent zombie processes.
diff --git a/src/haveged.c b/src/haveged.c
index e061079..4d05fdb 100644
--- a/src/haveged.c
+++ b/src/haveged.c
@@ -538,8 +538,10 @@ static void run_daemon( /* RETURN: nothing */
anchor_info(h);
return;
}
- if (params->foreground==0)
+ if (params->foreground==0) {
daemonize();
+ havege_reparent(handle);
+ }
else printf ("%s starting up\n", params->daemon);
if (0 != havege_run(h))
error_exit("Couldn't initialize HAVEGE rng %d", h->error);