fixeria has uploaded this change for review. ( https://gerrit.osmocom.org/c/erlang/osmo-s1gw/+/42450?usp=email )
Change subject: rest_server: fix TOC/TOU race when listing/fetching E-RABs ......................................................................
rest_server: fix TOC/TOU race when listing/fetching E-RABs
The list of E-RAB FSM pids is a snapshot taken at one point in time. By the time we interrogate each erab_fsm process individually, any of them may have already terminated (e.g. bearer released mid-request). The current code fails to generate a response if this happens.
* fetch_erab_info/1: add a pid() clause that wraps erab_list_item/1 in a try/catch, returning 'error' if the process is gone.
* fetch_erab_info/1: catch both exit forms ** `{noproc, _}` raised by gen_statem:call/2 on a monitored pid, and ** the bare noproc atom for other code paths.
* fetch_erab_list/1: switch from lists:map to lists:filtermap and call fetch_erab_info/1 per E-RAB, silently dropping any that died between the snapshot and the per-process interrogation.
Change-Id: I160b413aa535f2379ad4e40a3ae8f37c5bce2067 Related: SYS#7066 --- M src/rest_server.erl 1 file changed, 14 insertions(+), 6 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/erlang/osmo-s1gw refs/changes/50/42450/1
diff --git a/src/rest_server.erl b/src/rest_server.erl index bc9a4a9..cbdd6e6 100644 --- a/src/rest_server.erl +++ b/src/rest_server.erl @@ -523,14 +523,17 @@ error.
--spec fetch_erab_info(binary()) -> {ok, erab_fsm:erab_info()} | error. +-spec fetch_erab_info(binary() | pid()) -> {ok, erab_fsm:erab_info()} | error. fetch_erab_info(<< "pid:", Val/bytes >>) -> - Pid = parse_pid(Val), - %% guard against non-existent process IDs - %% TODO: check if the given Pid is actually an erab_fsm + fetch_erab_info(parse_pid(Val)); + +fetch_erab_info(Pid) when is_pid(Pid) -> + %% erab_fsm process may have already terminated + %% guard against that by catching noproc try erab_list_item({pid, Pid}) of ErabInfo -> {ok, ErabInfo} catch + exit:noproc -> error; exit:{noproc, _} -> error end;
@@ -542,12 +545,17 @@ -spec fetch_erab_list(enb_registry:enb_info()) -> [map()]. fetch_erab_list(#{pid := EnbPid}) -> ERABs = enb_proxy:fetch_erab_list(EnbPid), - lists:map(fun erab_list_item/1, ERABs). + lists:filtermap( + fun({_, Pid}) -> + case fetch_erab_info(Pid) of + {ok, ErabInfo} -> {true, ErabInfo}; + error -> false + end + end, ERABs).
-spec erab_list_item({term(), pid()}) -> map(). erab_list_item({_, Pid}) -> - %% XXX: E-RAB FSM process might be dead here Info = erab_fsm:fetch_info(Pid), {MmeUeId, ErabId} = maps:get(uid, Info), M0 = #{mme_ue_id => MmeUeId,