Re: [PATCH] sim: fix use-after-free in open_channel_cb watcher loop

On 5/7/26 19:02, Marius Gripsgard wrote:
> The notify callback (get_session_cb -> sim_fs_end_current) calls
> __ofono_sim_remove_session_watch, which removes and frees the current
> GSList node from session->watches->items. iter = g_slist_next(iter)
> then reads ->next from the freed node, on newer glibc the freed memory
> holds a random tcache key rather than accessible heap data, turning the
> next iter->data dereference into a SIGSEGV.
> 
> Advance iter before calling notify so the next-pointer is captured
> before the current node can be freed.
> ---
>   src/sim.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/src/sim.c b/src/sim.c
> index dedb7d8d..6a733169 100644
> --- a/src/sim.c
> +++ b/src/sim.c
> @@ -3659,9 +3659,9 @@ end:
>   		struct ofono_watchlist_item *item = iter->data;
>   		ofono_sim_session_event_cb_t notify = item->notify;
>   
> -		notify(active, session->session_id, item->notify_data);
> -
>   		iter = g_slist_next(iter);
> +
> +		notify(active, session->session_id, item->notify_data);
>   	}
>   }
>   


This fix looks both necessary (the error path in get_session_cb() will 
potentially call __ofono_sim_remove_session_watch() in its error path; 
and get_session_cb() is used as a notify callback and open_channel_cb() 
iterates unsafely through a GSList) and correct.

In general ofono should probably be calling g_slist_foreach() instead of 
manually iterating through lists to avoid this type of bug.