diff options
author | Sergei Trofimovich <slyfox@gentoo.org> | 2020-05-01 21:55:05 +0200 |
---|---|---|
committer | Sergei Trofimovich <slyfox@gentoo.org> | 2020-05-01 22:39:44 +0200 |
commit | 7526e9fe68c324599f0babb65915522d2a9a7dd7 (patch) | |
tree | a55ab06fa0e0e83316afae6930a40ed092879f31 /src | |
parent | Merge pull request #24 from Polynomial-C/enable-threads_build_fix (diff) | |
download | haveged-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.c | 13 | ||||
-rw-r--r-- | src/havege.h | 8 | ||||
-rw-r--r-- | src/haveged.c | 4 |
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); |