Change in ...libosmocore[master]: add osmo_fsm_inst_watch()

[neels]

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 <nhofmeyr at sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-MessageType: newpatchset
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20191002/812e31ab/attachment.htm>