Change in libosmocore[master]: add osmo_fsm_set_dealloc_ctx(), to help with use-after-free

This is merely a historical archive of years 2008-2021, before the migration to mailman3.

A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/gerrit-log@lists.osmocom.org/.

neels gerrit-no-reply at lists.osmocom.org
Tue Oct 29 16:14:03 UTC 2019


neels has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/15677 )

Change subject: add osmo_fsm_set_dealloc_ctx(), to help with use-after-free
......................................................................


Patch Set 2:

Async events would introduce pretty large complexity, and they would not solve the use-after-free problems.

What these FSM patches do is remove complexity.

The beauty of it is that plain function calls benefit from it just like other FSM instances do.

This is another instance of a feature that should have been enabled from the start, for very simple and straightforward reasons: when an FSM instance terminates, how do I check whether it has terminated? It is a paradox, because when the instance has terminated, I am not allowed to access its memory anymore (to check a flag or fi->state), because I would access freed memory. I have had to solve countless crashes because of this fundamental problem.

This problem has been present right from the start of osmo_fsm, and for ages I have been fighting these problems with super complex structures of conditionals that skip cleaning up or not, depending on what might have happened or some specialized flags. That was super annoying BF hell. A lot of wasted time.

If instead deallocation is deferred, we can easily check a flag like fi->proc.terminating or fi->state without risking a use-after-free. We can also implicitly stop event dispatch and state transitions and duplicate term. We can solve this entire class of cleanup problems with one elegant solution, which fixeria came up with, btw, which I am super thankful for.

For me this and osmo_fsm_set_term_stops_actions() are a game changer in implementing FSM cleanup, all I see is a massive bunch of complexity falling away and leaving behind very simple cleanup logic. I really cracked my head on this stuff over the past years, and now all of it is just solved without adverse effects. So I am actually surprised that you guys see it as kludge and complexity or nuclear weapons on flies, it is the opposite. It is a super simple solution for huuuuugely complex problems, not one, but all of them, in one fell swoop.

Maybe you guys don't appreciate it as much as I do, since I believe I am the person that has spent the most time connecting various FSM instances running in complex relations to each other. Even in plain parent/child associations, I needed the osmo_fsm_term_safely() to avoid use-after-free, which happens as soon as both a parent and a child want to trigger collective term. (If I could go back, I would actually undo osmo_fsm_term_safely() and rather use this more elegant solution instead.)

If you doubt the necessity of solving these problems, just switch off osmo_fsm_term_safely() and this patch, and try to solve the problems in fsm_dealloc_test.c manually, see libosmocore/tests/fsm/.


-- 
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/15677
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings

Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: Ief4dba9ea587c9b4aea69993e965fbb20fb80e78
Gerrit-Change-Number: 15677
Gerrit-PatchSet: 2
Gerrit-Owner: neels <nhofmeyr at sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <axilirator at gmail.com>
Gerrit-Reviewer: neels <nhofmeyr at sysmocom.de>
Gerrit-CC: laforge <laforge at osmocom.org>
Gerrit-CC: pespin <pespin at sysmocom.de>
Gerrit-Comment-Date: Tue, 29 Oct 2019 16:14:03 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20191029/d42033dd/attachment.htm>


More information about the gerrit-log mailing list