> 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:

        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?

-- 
You received this bug notification because you are a member of Ubuntu
Bugs, which is subscribed to Ubuntu.
https://bugs.launchpad.net/bugs/1750013

Title:
  systemd-logind: memory leaks on session's connections (trusty-only)

To manage notifications about this bug go to:
https://bugs.launchpad.net/ubuntu/+source/systemd/+bug/1750013/+subscriptions

-- 
ubuntu-bugs mailing list
ubuntu-bugs@lists.ubuntu.com
https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs

Reply via email to