Hello Jenkins Builder, I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/c/libosmocore/+/15660 to look at the new patch set (#2). Change subject: add osmo_fsm_inst_watch() ...................................................................... add osmo_fsm_inst_watch() I discovered an osmo-msc use-after-free crash from an invalid message, caused by this pattern: void event_action() { osmo_fsm_inst_dispatch(foo, FOO_EVENT, NULL); osmo_fsm_inst_dispatch(bar, BAR_EVENT, NULL); } Usually, FOO_EVENT takes successful action, and afterwards we also notify bar of another event. However, in this particular case FOO_EVENT caused failure, and the immediate error handling directly terminated and deallocated bar. In such cases, dispatching BAR_EVENT causes a use-after-free; this constituted a DoS vector just from sending messages that fail to validate to osmo-msc. Instead, introduce this pattern for accessing FSM instances after failure-critical actions, which watches out for a given osmo_fsm_inst's deallocation: void event_action() { struct osmo_fsm_inst_watcher watch_bar; osmo_fsm_inst_watch(&watch_bar, bar); osmo_fsm_inst_dispatch(foo, FOO_EVENT, NULL); osmo_fsm_inst_unwatch(&watch_bar); if (watch_bar.exists) osmo_fsm_inst_dispatch(bar, BAR_EVENT, NULL); } Implementation: at first I had thought of a simple lookup whether bar still is listed in the bar->fsm list of osmo_fsm_inst instances. That worked well, but theoretically, after a deallocation, another FSM may have been allocated, possibly at the exact same memory address. This chance is slim, but nevertheless quite possible. The only fully safe way is to explicitly watch an instance. Test: incorporate FSM instance watchers in fsm_dealloc_test.c, with OSMO_ASSERTs verifying that the watchers reflect exactly whether an object is still allocated. Though the test's expected output does not print anything when the osmo_fsm_inst_watchers reflect the expected values, I did verify that the test catches bugs when introduced deliberately. Related: Iaa8e3da2969ebb4c78bff11d0d59f01b10f341d7 (osmo-msc), I790d2172c7c67610915cb74b0766fb18f2795b29 (osmo-mgw) Change-Id: I4d8306488506c60b4c2fc1c4cb3ac04654db9c43 --- M include/osmocom/core/fsm.h M src/fsm.c M tests/fsm/fsm_dealloc_test.c 3 files changed, 116 insertions(+), 1 deletion(-) git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/60/15660/2 -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/15660 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I4d8306488506c60b4c2fc1c4cb3ac04654db9c43 Gerrit-Change-Number: 15660 Gerrit-PatchSet: 2 Gerrit-Owner: neels <nhofm...@sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-MessageType: newpatchset