summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorErlang/OTP <otp@erlang.org>2021-09-02 12:15:48 +0200
committerErlang/OTP <otp@erlang.org>2021-09-02 12:15:48 +0200
commit715b7e564ea5fbed52b52400919c0c9fc3e57dc1 (patch)
treed47684075e2ebe186b51c6334b12cb5394df2c55
parent8cc5dd225238370528edf2c159dcd43695972f9d (diff)
parenteb89e5eb5c789826480cb79c17b06588838cc41f (diff)
downloaderlang-715b7e564ea5fbed52b52400919c0c9fc3e57dc1.tar.gz
Merge branch 'lukas/erts/fix-erl_tracer-use-after-free/PR-4940/OTP-17568' into maint-22
* lukas/erts/fix-erl_tracer-use-after-free/PR-4940/OTP-17568: erts: Fix use after free for ErtsTracerNif* # Conflicts: # erts/emulator/test/dirty_nif_SUITE.erl # erts/emulator/test/gc_SUITE.erl
-rw-r--r--erts/emulator/beam/erl_nif.c2
-rw-r--r--erts/emulator/beam/erl_process.c9
-rw-r--r--erts/emulator/beam/erl_trace.c3
-rw-r--r--erts/emulator/beam/module.c1
-rw-r--r--erts/emulator/test/dirty_nif_SUITE.erl29
-rw-r--r--erts/emulator/test/gc_SUITE.erl33
6 files changed, 63 insertions, 14 deletions
diff --git a/erts/emulator/beam/erl_nif.c b/erts/emulator/beam/erl_nif.c
index 87e469dd98..1385332282 100644
--- a/erts/emulator/beam/erl_nif.c
+++ b/erts/emulator/beam/erl_nif.c
@@ -880,6 +880,8 @@ int enif_send(ErlNifEnv* env, const ErlNifPid* to_pid,
from = c_p ? c_p->common.id : am_undefined;
if (!env || !env->tracee) {
+ /* This clause is taken when enif_send is called in a nif
+ that is not a erl_tracer nif. */
if (c_p && IS_TRACED_FL(c_p, F_TRACE_SEND)) {
full_flush_env(env);
diff --git a/erts/emulator/beam/erl_process.c b/erts/emulator/beam/erl_process.c
index 0c8999dc16..da2a5b8904 100644
--- a/erts/emulator/beam/erl_process.c
+++ b/erts/emulator/beam/erl_process.c
@@ -10080,9 +10080,12 @@ Process *erts_schedule(ErtsSchedulerData *esdp, Process *p, int calls)
static void
trace_schedule_in(Process *p, erts_aint32_t state)
{
+ ErtsThrPrgrDelayHandle dhndl;
ASSERT(IS_TRACED(p));
ERTS_LC_ASSERT(erts_proc_lc_my_proc_locks(p) == ERTS_PROC_LOCK_MAIN);
+ dhndl = erts_thr_progress_unmanaged_delay();
+
/* Clear tracer if it has been removed */
if (erts_is_tracer_proc_enabled(p, ERTS_PROC_LOCK_MAIN, &p->common)) {
@@ -10098,14 +10101,17 @@ trace_schedule_in(Process *p, erts_aint32_t state)
if (IS_TRACED_FL(p, F_TRACE_CALLS))
erts_schedule_time_break(p, ERTS_BP_CALL_TIME_SCHEDULE_IN);
}
-
+ erts_thr_progress_unmanaged_continue(dhndl);
}
static void
trace_schedule_out(Process *p, erts_aint32_t state)
{
+ ErtsThrPrgrDelayHandle dhndl;
ASSERT(IS_TRACED(p));
ERTS_LC_ASSERT(erts_proc_lc_my_proc_locks(p) == ERTS_PROC_LOCK_MAIN);
+
+ dhndl = erts_thr_progress_unmanaged_delay();
if (IS_TRACED_FL(p, F_TRACE_CALLS) && !(state & ERTS_PSFLG_FREE))
erts_schedule_time_break(p, ERTS_BP_CALL_TIME_SCHEDULE_OUT);
@@ -10119,6 +10125,7 @@ trace_schedule_out(Process *p, erts_aint32_t state)
ARE_TRACE_FLAGS_ON(p, F_TRACE_SCHED_PROCS))
trace_sched(p, ERTS_PROC_LOCK_MAIN, am_out);
}
+ erts_thr_progress_unmanaged_continue(dhndl);
}
static int
diff --git a/erts/emulator/beam/erl_trace.c b/erts/emulator/beam/erl_trace.c
index 7104132320..7660f19966 100644
--- a/erts/emulator/beam/erl_trace.c
+++ b/erts/emulator/beam/erl_trace.c
@@ -1348,6 +1348,7 @@ trace_gc(Process *p, Eterm what, Uint size, Eterm msg)
Eterm* hp;
Uint sz = 0;
Eterm tup;
+ ErtsThrPrgrDelayHandle dhndl = erts_thr_progress_unmanaged_delay();
if (is_tracer_enabled(p, ERTS_PROC_LOCK_MAIN, &p->common, &tnif,
TRACE_FUN_E_GC, what)) {
@@ -1367,6 +1368,7 @@ trace_gc(Process *p, Eterm what, Uint size, Eterm msg)
if (o_hp)
erts_free(ERTS_ALC_T_TMP, o_hp);
}
+ erts_thr_progress_unmanaged_continue(dhndl);
}
void
@@ -2617,6 +2619,7 @@ lookup_tracer_nif(const ErtsTracer tracer)
ErtsTracerNif tnif_tmpl;
ErtsTracerNif *tnif;
tnif_tmpl.module = ERTS_TRACER_MODULE(tracer);
+ ERTS_LC_ASSERT(erts_thr_progress_lc_is_delaying() || erts_get_scheduler_id() > 0);
erts_rwmtx_rlock(&tracer_mtx);
if ((tnif = hash_get(tracer_hash, &tnif_tmpl)) == NULL) {
erts_rwmtx_runlock(&tracer_mtx);
diff --git a/erts/emulator/beam/module.c b/erts/emulator/beam/module.c
index 0642a06123..08d4f26d8d 100644
--- a/erts/emulator/beam/module.c
+++ b/erts/emulator/beam/module.c
@@ -133,6 +133,7 @@ erts_get_module(Eterm mod, ErtsCodeIndex code_ix)
IndexTable* mod_tab;
ASSERT(is_atom(mod));
+ ERTS_LC_ASSERT(erts_get_scheduler_id() > 0 || erts_thr_progress_lc_is_delaying());
mod_tab = &module_tables[code_ix];
diff --git a/erts/emulator/test/dirty_nif_SUITE.erl b/erts/emulator/test/dirty_nif_SUITE.erl
index 71cfb3f944..fc4c9a514a 100644
--- a/erts/emulator/test/dirty_nif_SUITE.erl
+++ b/erts/emulator/test/dirty_nif_SUITE.erl
@@ -447,10 +447,10 @@ dirty_nif_send_traced(Config) when is_list(Config) ->
{ok, Rcvr} = send_wait_from_dirty_nif(Rcvr),
Parent ! {self(), sent}
end),
- 1 = erlang:trace(Sndr, true, [send]),
+ 1 = erlang:trace(Sndr, true, [send, running, exiting]),
Start = erlang:monotonic_time(),
Sndr ! {self(), go},
- receive {trace, Sndr, send, {ok, Rcvr}, Rcvr} -> ok end,
+
receive {Rcvr, received} -> ok end,
End1 = erlang:monotonic_time(),
Time1 = erlang:convert_time_unit(End1-Start, native, 1000),
@@ -461,8 +461,33 @@ dirty_nif_send_traced(Config) when is_list(Config) ->
Time2 = erlang:convert_time_unit(End2-Start, native, 1000),
io:format("Time2: ~p milliseconds~n", [Time2]),
true = Time2 >= 1900,
+
+ %% Make sure that the send trace is
+ %% in between an in and and out trace
+ (fun F() ->
+ %% We got an in trace, look for out or send
+ {trace,Sndr,in,_} = recv_trace_from(Sndr),
+ case recv_trace_from(Sndr) of
+ {trace,Sndr,out,_} ->
+ %% We got an out, look for another in
+ F();
+ {trace,Sndr,send,_,_} ->
+ %% We got a send, look for another out
+ {trace,Sndr,out,_} = recv_trace_from(Sndr),
+ ok
+ end
+ end)(),
+
ok.
+recv_trace_from(Sndr) ->
+ receive
+ M when element(1, M) =:= trace;
+ element(1, M) =:= trace_ts,
+ element(2, M) =:= Sndr ->
+ M
+ end.
+
dirty_literal_test_code() ->
"
-module(dirty_literal_code_test).
diff --git a/erts/emulator/test/gc_SUITE.erl b/erts/emulator/test/gc_SUITE.erl
index 810409175e..cdeec41d5f 100644
--- a/erts/emulator/test/gc_SUITE.erl
+++ b/erts/emulator/test/gc_SUITE.erl
@@ -211,6 +211,13 @@ minor_major_gc_option_self(_Config) ->
Pid ! {gc, Ref, major}
end, [gc_major_start, gc_major_end]),
+ %% Try as major dirty, the test process will self-trigger GC
+ check_gc_tracing_around(
+ fun(Pid, Ref) ->
+ Pid ! {gc, Ref, major}
+ end, [gc_major_start, gc_major_end],
+ lists:seq(1,128 * 1024)),
+
%% Try as minor, the test process will self-trigger GC
check_gc_tracing_around(
fun(Pid, Ref) ->
@@ -291,19 +298,23 @@ gc_dirty_exec_proc(Config) when is_list(Config) ->
%% it results in any unexpected messages or if the expected trace tags are not
%% received.
check_gc_tracing_around(Fun, ExpectedTraceTags) ->
+ check_gc_tracing_around(Fun, ExpectedTraceTags, []).
+check_gc_tracing_around(Fun, ExpectedTraceTags, State) ->
Ref = erlang:make_ref(),
Pid = spawn(
- fun Endless() ->
- receive
- {gc, Ref, Type} ->
- erlang:garbage_collect(self(), [{type, Type}]);
- {dirty_exec, Time} ->
- erts_debug:dirty_io(wait, 1000)
- after 100 ->
- ok
- end,
- Endless()
- end),
+ fun() ->
+ (fun Endless(S) ->
+ receive
+ {gc, Ref, Type} ->
+ erlang:garbage_collect(self(), [{type, Type}]);
+ {dirty_exec, Time} ->
+ erts_debug:dirty_io(wait, Time)
+ after 100 ->
+ ok
+ end,
+ Endless(S)
+ end)(State)
+ end),
erlang:garbage_collect(Pid, []),
erlang:trace(Pid, true, [garbage_collection]),
Fun(Pid, Ref),