Skip to content
Snippets Groups Projects
Commit aa360b25 authored by Alexander Hansen Færøy's avatar Alexander Hansen Færøy
Browse files

Fix crash bug in PT subsystem.

This patch fixes a crash bug (assertion failure) in the PT subsystem
that could get triggered if the user cancels bootstrap via the UI in
TorBrowser. This would cause Tor to call `managed_proxy_destroy()` which
called `process_free()` after it had called `process_terminate()`. This
leads to a crash when the various process callbacks returns with data
after the `process_t` have been freed using `process_free()`.

We solve this issue by ensuring that everywhere we call
`process_terminate()` we make sure to detach the `managed_proxy_t` from
the `process_t` (by calling `process_set_data(process, NULL)`) and avoid
calling `process_free()` at all in the transports code. Instead we just
call `process_terminate()` and let the process exit callback in
`managed_proxy_exit_callback()` handle the `process_free()` call by
returning true to the process subsystem.

See: https://bugs.torproject.org/29562
parent 1bff5646
No related branches found
No related tags found
No related merge requests found
o Minor bugfixes (pluggable transports):
- Fix an assertion failure crash bug when a pluggable transport process is
terminated during the bootstrap phase. Fixes bug 29562; bugfix on
0.4.0.1-alpha.
......@@ -713,10 +713,13 @@ managed_proxy_destroy(managed_proxy_t *mp,
tor_free(mp->proxy_uri);
/* do we want to terminate our process if it's still running? */
if (also_terminate_process && mp->process)
if (also_terminate_process && mp->process) {
/* Note that we do not call process_free(mp->process) here because we let
* the exit handler in managed_proxy_exit_callback() return `true` which
* makes the process subsystem deallocate the process_t. */
process_set_data(mp->process, NULL);
process_terminate(mp->process);
process_free(mp->process);
}
tor_free(mp);
}
......@@ -1823,6 +1826,9 @@ managed_proxy_stdout_callback(process_t *process,
managed_proxy_t *mp = process_get_data(process);
if (BUG(mp == NULL))
return;
handle_proxy_line(line, mp);
if (proxy_configuration_finished(mp)) {
......@@ -1846,6 +1852,9 @@ managed_proxy_stderr_callback(process_t *process,
managed_proxy_t *mp = process_get_data(process);
if (BUG(mp == NULL))
return;
log_warn(LD_PT, "Managed proxy at '%s' reported: %s", mp->argv[0], line);
}
......@@ -1862,18 +1871,8 @@ managed_proxy_exit_callback(process_t *process, process_exit_code_t exit_code)
"Pluggable Transport process terminated with status code %" PRIu64,
exit_code);
/* We detach ourself from the MP (if we are attached) and free ourself. */
managed_proxy_t *mp = process_get_data(process);
/* If we are still attached to the process, it is probably because our PT
* process crashed before we got to call process_set_data(p, NULL); */
if (BUG(mp != NULL)) {
/* FIXME(ahf): Our process stopped without us having told it to stop
* (crashed). Should we restart it here? */
mp->process = NULL;
process_set_data(process, NULL);
}
/* Returning true here means that the process subsystem will take care of
* calling process_free() on our process_t. */
return true;
}
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment