fixeria has uploaded this change for review.

View Change

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 change 42450. To unsubscribe, or for help writing mail filters, visit settings.