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,
--
To view, visit https://gerrit.osmocom.org/c/erlang/osmo-s1gw/+/42450?usp=email
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: newchange
Gerrit-Project: erlang/osmo-s1gw
Gerrit-Branch: master
Gerrit-Change-Id: I160b413aa535f2379ad4e40a3ae8f37c5bce2067
Gerrit-Change-Number: 42450
Gerrit-PatchSet: 1
Gerrit-Owner: fixeria <[email protected]>
