-
-
Notifications
You must be signed in to change notification settings - Fork 3.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
logind: register PAM sessions via Varlink instead of D-Bus #35264
base: main
Are you sure you want to change the base?
Conversation
_cleanup_close_ int fifo_fd = session_create_fifo(s); | ||
if (fifo_fd < 0) | ||
return fifo_fd; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So, hmm, I'm quite inclined to just leave this whole fifo stuff behind when we're embracing the new varlink and pidfd world. We actually stop the session as soon as the session leader exits when listening on pidfd, so the fifo is useless here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmm, right now pidfd (and in particular SO_PEERPIDFD is not in our kernel baseline yet. I am not sure we can do this already.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm going to bump our baseline to v5.4 in v258, so I'd really appreciate it if we just send logind our pidfd, and drop the fifo concept altogether (I can handle the latter part later)
4baee2d
to
67591fd
Compare
67591fd
to
34e18bc
Compare
The new json_dispatch_const_path() is to json_dispatch_path() what sd_json_dispatch_const_string() is to sd_json_dispatch_ string(), i.e. doesn't implicitly strdup() the string, but gives you the pointer into the JSON structure, and thus requires you to keep it pinned.
…d_get_owner_uid()
This combines getpeercred() and getpeerpidfd() and returns a PidRef
…ith fd passing enabled Let's add a simple flag that enables fd passing for all connections of a server. It's much easier to use this than to install a connect handler which manually enables this for each connection.
We can pass a properly typed Manager object here, no reason to pass it as void*.
Just some refactoring to make an overly large function a bit smaller.
…k codepaths This separates the preparatory checks that generate D-Bus errors from the code that actually allocates the session. This make the logic easier to follow and prepares ground so that we can reuse the 2nd part later when exposing session creation via Varlink.
This prepares ground so that later on we can reply with either D-Bus or Varlink depending on the client's request.
For now this only covers CreateSession() and ReleaseSession(), i.e. the two operations pam_systemd cares about.
This makes sure we now use Varlink per default as transport for allocating sessions. This reduces the time it takes to do one run0 cycle by roughly ~10% on my completely synthetic test setup (assuming the target user's service manager is already started) The D-Bus codepaths are kept in place for two reasons: * To make upgrades easy * If the user actually sets resource properties on the PAM session we fall back to the D-Bus codepaths, as we currently have no way to encode the scope properties in JSON, this is only supported for D-Bus serialization. The latter should be revisited once it is possible to allocate a scope unit from PID1 via varlink.
34e18bc
to
0bf70e5
Compare
This makes things a bit faster (because it cuts down a bit on roundtrips) and prepares ground so that one day we can let logind run in earlier boot already, making it a bit less special.
communication between logind and pid1 is still dbus only, hence there's a lot of room for further improvement I guess.