> when SSH ends, for example, a Release event is sent
> through dbus and systemd-logind captures it, in the function manager_message_handler().
>
> From there, the function session_remove_fifo() is called. That point is our "bootstrap"
> to add the closing session on gc
ok i see that path, dbus Release -> remove fifo, however i don't think it's appropriate to put the add_to_gc into remove_fifo; because, remove_fifo is also called from session_free()..consider this code:
session_remove_fifo(s);
free(s->state_file);
free(s);
}
so...that call to session_remove_fifo() previously only removed the fifo. but with your patch, it's also adding 's' back onto the gc queue...but then immediately freeing s!!! When the gc queue is processed, this code:
will dereference 's' ('session' in the gc handler), accessing freed memory.
Since you're trying to use the dbus Release notification to add the session to the gc, wouldn't it be better to simply update the dbus Release handler to remove the fifo *and* add the session to the gc, from there? not modify the remove_fifo function?
> when SSH ends, for example, a Release event is sent message_ handler( ). remove_ fifo() is called. That point is our "bootstrap"
> through dbus and systemd-logind captures it, in the function manager_
>
> From there, the function session_
> to add the closing session on gc
ok i see that path, dbus Release -> remove fifo, however i don't think it's appropriate to put the add_to_gc into remove_fifo; because, remove_fifo is also called from session_ free(). .consider this code:
free(s);
}
so...that call to session_ remove_ fifo() previously only removed the fifo. but with your patch, it's also adding 's' back onto the gc queue...but then immediately freeing s!!! When the gc queue is processed, this code:
while ((session = m->session_ gc_queue) ) {
LIST_ REMOVE( Session, gc_queue, m->session_ gc_queue, session);
session- >in_gc_ queue = false;
will dereference 's' ('session' in the gc handler), accessing freed memory.
Since you're trying to use the dbus Release notification to add the session to the gc, wouldn't it be better to simply update the dbus Release handler to remove the fifo *and* add the session to the gc, from there? not modify the remove_fifo function?