Bluetooth: LLCP: possible access to released control procedure context

[ppryga-nordic]

Describe the bug https://github.com/zephyrproject-rtos/zephyr/blob/f8031c7ab08c7b27a3a1949ae6bcad6f9d45dbeb/subsys/bluetooth/controller/ll_sw/ull_llcp_local.c#L390-L391 The link points to code that executes top LLCP state machine.

Just after lr_act_run there is an if statement that verifies ctx->proc. Unfortunately there is a possibility that the ctx is released during processing of lr_act_run.

Follow example execution path for DLE procedure: ull_cp_run -> llcp_lr_run -> lr_execute_fsm -> lr_st_idle -> lr_act_run -> llcp_lp_comm_run -> lp_comm_execute_fsm -> lp_comm_st_idle -> lp_comm_send_req -> llcp_lr_complete (if remote DLE is pending) -> lr_execute_fsm(conn, LR_EVT_COMPLETE, NULL); -> lr_st_idle -> lr_act_complete (sets ctx->done = 1U -> returns to back to lr_act_run where at end lr_check_done releases the ctx -> returns to lr_st_idle where ctx is released and in the if statement there is use of it without checking if it is valid.

Expected behavior As of now it looks like there is no problem during execution because contexts are allocated in static memory, hence release does not make the memory to be overwritten. Also contexts are not being allocated immediately. In future it may happen that the context is re-used and stored information overwritten. It may lead to unexpected behavior that may be hard to find.

There should be a verification when there is a possibility that contexts are released before they are used again.

Impact Related only with refactored LLCPs.

[kruithofa]

I haven't found a good way of writing a unittest for this. After code inspection I found no other places where this issue pops up