osmo_fsm timer_cb fudge up

I'm finding heap use after free related to osmo_fsm: - When dispatching a timer_cb event, we want to set fi->T back to 0. (see fsm_tmr_cb()) - So that we know which timer fired, we leave fi->T at its current value for the timer_cb to be able to make decisions based on that. This is used for example in fsm_timeout_cb() in osmo-bsc/osmo_bsc_mgcp.c. - In other code, the timer_cb event may decide to fire events or call functions which effect an FSM instance teardown and deallocation, meaning that in fsm_tmr_cb(), we are *not* allowed to generally assume the fi still exists. If we set fi->T = 0 after the timer_cb() was called, we may write to freed mem. - Also, if the timer_cb effects events with their own timeout, we will also overwrite the fi->T value with zero after another timer has set its value in T, causing failures to evaluate that timer later on. Solutions so far: - set fi->T = 0 first, and pass T to the timer_cb as arg instead. I think this is the best and cleanest, but also the hardest in terms of backwards compat. I have a patch that adds timer_cb2() that features a T arg; we need to submit that, move all code that wants to use T to that timer_cb2(), and then we can move the fi->T = 0 to before the timer_cb(). Actually, this is not being backwards compatible at all, because all code trying to use fi->T as usual will need adjustment, we break API either way and might as well add T to the timer_cb directly instead... :/ - Actually not set fi->T = 0 at all. We will have T holding the value of the timer that fired last, all old code will still find this T in fi->T, and after the cb, we just leave it as it was. Does it really need to be cleared? This has no compatibility issues and solves the use after free as well as setting a new T from timer_cb() issues. Going for this one so far. Thoughts? ~N