diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index cc9e5b9..8b2a20d 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -15,7 +15,7 @@ jobs: image: erlang:${{matrix.otp_vsn}} strategy: matrix: - otp_vsn: ['20.3', '21.3', '22.3', '23.3', '24.3', '25.3', '26.0.2', '27.0.1'] + otp_vsn: ['20.3', '21.3', '22.3', '23.3', '24.3', '25.3', '26.0.2', '27.0.1', '28.4'] os: [ubuntu-latest] steps: - uses: actions/checkout@v2 diff --git a/src/recon_lib.erl b/src/recon_lib.erl index 8af2bfa..0734a3a 100644 --- a/src/recon_lib.erl +++ b/src/recon_lib.erl @@ -18,6 +18,8 @@ %% private exports -export([binary_memory/1]). +-dialyzer({nowarn_function, [fold_processes/2, fold_processes_iter/3]}). + -type diff() :: [recon:proc_attrs() | recon:inet_attrs()]. %% @doc Compare two samples and return a list based on some key. The type mentioned diff --git a/src/recon_trace.erl b/src/recon_trace.erl index 62b3b67..b0b3b6e 100644 --- a/src/recon_trace.erl +++ b/src/recon_trace.erl @@ -337,13 +337,15 @@ calls(TSpecs = [_|_], Max) -> calls({Mod, Fun, Args}, Max, Opts) -> calls([{Mod,Fun,Args}], Max, Opts); calls(TSpecs = [_|_], {Max, Time}, Opts) -> + IOServer = validate_io_server(Opts), Pid = setup(rate_tracer, [Max, Time], - validate_formatter(Opts), validate_io_server(Opts)), - trace_calls(TSpecs, Pid, Opts); + validate_formatter(Opts), IOServer), + trace_calls(TSpecs, Pid, IOServer, Opts); calls(TSpecs = [_|_], Max, Opts) -> + IOServer = validate_io_server(Opts), Pid = setup(count_tracer, [Max], - validate_formatter(Opts), validate_io_server(Opts)), - trace_calls(TSpecs, Pid, Opts). + validate_formatter(Opts), IOServer), + trace_calls(TSpecs, Pid, IOServer, Opts). %%%%%%%%%%%%%%%%%%%%%%% %%% PRIVATE EXPORTS %%% @@ -421,10 +423,12 @@ setup(TracerFun, TracerArgs, FormatterFun, IOServer) -> end. %% Sets the traces in action -trace_calls(TSpecs, Pid, Opts) -> +trace_calls(TSpecs, Pid, IOServer, Opts) -> {PidSpecs, TraceOpts, MatchOpts} = validate_opts(Opts), + UnsafePids = build_unsafe_pids(Pid, IOServer), + PidsOnly = lists:all(fun(P) -> is_safe_pid(P, UnsafePids) end, PidSpecs), Matches = [begin - {Arity, Spec} = validate_tspec(Mod, Fun, Args), + {Arity, Spec} = validate_tspec(Mod, Fun, Args, PidsOnly), erlang:trace_pattern({Mod, Fun, Arity}, Spec, MatchOpts) end || {Mod, Fun, Args} <- TSpecs], [erlang:trace(PidSpec, true, [call, {tracer, Pid} | TraceOpts]) @@ -480,18 +484,19 @@ validate_pid_specs(PidTerm) -> %% has to be `recon:pid_term()'. [recon_lib:term_to_pid(PidTerm)]. -validate_tspec(Mod, Fun, Args) when is_function(Args) -> - validate_tspec(Mod, Fun, fun_to_ms(Args)); +validate_tspec(Mod, Fun, Args, PidsOnly) when is_function(Args) -> + validate_tspec(Mod, Fun, fun_to_ms(Args), PidsOnly); %% helper to save typing for common actions -validate_tspec(Mod, Fun, return_trace) -> - validate_tspec(Mod, Fun, [{'_', [], [{return_trace}]}]); -validate_tspec(Mod, Fun, Args) -> +validate_tspec(Mod, Fun, return_trace, PidsOnly) -> + validate_tspec(Mod, Fun, [{'_', [], [{return_trace}]}], PidsOnly); +validate_tspec(Mod, Fun, Args, PidsOnly) -> BannedMods = ['_', ?MODULE, io, lists], %% The banned mod check can be bypassed by using %% match specs if you really feel like being dumb. - case {lists:member(Mod, BannedMods), Args} of + case {lists:member(Mod, BannedMods) andalso not PidsOnly, Args} of {true, '_'} -> error({dangerous_combo, {Mod,Fun,Args}}); {true, []} -> error({dangerous_combo, {Mod,Fun,Args}}); + {true, [{'_', [], [{return_trace}]}]} -> error({dangerous_combo, {Mod,Fun,Args}}); _ -> ok end, case Args of @@ -500,6 +505,17 @@ validate_tspec(Mod, Fun, Args) -> _ when Args >= 0, Args =< 255 -> {Args, true} end. +is_safe_pid(P, Unsafe) when is_pid(P) -> not lists:member(P, Unsafe); +is_safe_pid(_, _) -> false. + +resolve_io_server(Pid) when is_pid(Pid) -> Pid; +resolve_io_server(Name) when is_atom(Name) -> whereis(Name). + +build_unsafe_pids(TracerPid, IOServer) -> + IOPid = resolve_io_server(IOServer), + FormatterPid = whereis(recon_trace_formatter), + [P || P <- [self(), TracerPid, FormatterPid, IOPid], is_pid(P)]. + validate_formatter(Opts) -> case proplists:get_value(formatter, Opts) of F when is_function(F, 1) -> F; diff --git a/test/recon_trace_SUITE.erl b/test/recon_trace_SUITE.erl new file mode 100644 index 0000000..3e819f6 --- /dev/null +++ b/test/recon_trace_SUITE.erl @@ -0,0 +1,52 @@ +-module(recon_trace_SUITE). +-include_lib("common_test/include/ct.hrl"). +-include_lib("eunit/include/eunit.hrl"). +-compile(export_all). + +all() -> [dangerous_combo, dangerous_combo_specific_pids, + dangerous_combo_self_pid, dangerous_combo_io_server_pid]. + +%%%%%%%%%%%%% +%%% TESTS %%% +%%%%%%%%%%%%% + +dangerous_combo(_Config) -> + ?assertError({dangerous_combo, {'_', '_', '_'}}, + recon_trace:calls([{'_', '_', '_'}], 1)), + ?assertError({dangerous_combo, {'_', '_', []}}, + recon_trace:calls([{'_', '_', []}], 1)), + ?assertError({dangerous_combo, {io, '_', []}}, + recon_trace:calls([{io, '_', []}], 1)), + ?assertError({dangerous_combo, {lists, '_', []}}, + recon_trace:calls([{lists, '_', []}], 1)), + ?assertError({dangerous_combo, {'_', '_', [{'_', [], [{return_trace}]}]}}, + recon_trace:calls([{'_', '_', return_trace}], 1)), + ?assertError({dangerous_combo, {io, '_', [{'_', [], [{return_trace}]}]}}, + recon_trace:calls([{io, '_', return_trace}], 1)), + ?assertError({dangerous_combo, {lists, '_', [{'_', [], [{return_trace}]}]}}, + recon_trace:calls([{lists, '_', return_trace}], 1)), + ?assertError({dangerous_combo, {recon_trace, '_', [{'_', [], [{return_trace}]}]}}, + recon_trace:calls([{recon_trace, '_', return_trace}], 1)). + +dangerous_combo_specific_pids(_Config) -> + Pid = spawn(fun() -> timer:sleep(infinity) end), + Pid2 = spawn(fun() -> timer:sleep(infinity) end), + try + recon_trace:calls([{'_', '_', '_'}], 1, [{pid, Pid}]), + recon_trace:calls([{'_', '_', '_'}], 1, [{pid, [Pid, Pid2]}]), + recon_trace:clear() + after + exit(Pid, kill), + exit(Pid2, kill) + end, + ?assertError({dangerous_combo, {'_', '_', '_'}}, + recon_trace:calls([{'_', '_', '_'}], 1, [{pid, [new, Pid, Pid2]}])). + +dangerous_combo_self_pid(_Config) -> + ?assertError({dangerous_combo, {'_', '_', '_'}}, + recon_trace:calls([{'_', '_', '_'}], 1, [{pid, self()}])). + +dangerous_combo_io_server_pid(_Config) -> + GL = group_leader(), + ?assertError({dangerous_combo, {'_', '_', '_'}}, + recon_trace:calls([{'_', '_', '_'}], 1, [{pid, GL}])).