some nss deadlock love #9504
some nss deadlock love #9504
Conversation
doc/ENVIRONMENT.md
Outdated
|
|
||
| systemd itself: | ||
|
|
||
| * `$SYSTEMD_ACTIVATION_UNIT` — set for all NSS and PAM module invocatins that |
There was a problem hiding this comment.
s/invocatins/invocations/
Also, do you want to codify here that the value is the name of the parent unit? (I'm not sure if it is already implicit now or intentionally left undefined)
There was a problem hiding this comment.
hmm, what do you mean by "parent" unit? it's the unit that we run the NSS/PAM lookups for.
There was a problem hiding this comment.
I was only asking if you want to specify in this doc what the value for the environment flag is supposed to be (as the modules may want to check for specific values, like this PR does).
There was a problem hiding this comment.
This looks good in general, apart from the minor types.
Nevertheless, I'm starting to think that DynamicUser=yes for basic services like systemd-resolved and systemd-networkd is not the right solution. Dynamic users make a lot of sense for transient services or for stuff which is not always running. But for stuff that we except to be always there, and running on a significant percentage of installations, removes the gains we get. Am I missing some important advantage apart from giving the DynamicUser implementation more testing?
src/core/execute.c
Outdated
| @@ -2828,10 +2828,22 @@ static int exec_child( | |||
| } | |||
| } | |||
|
|
|||
| /* We are about to invoke NSS and PAM modules. Let's tell them what we are doing here, maybe they care. This is | |||
| * used by nss-resolve to disabled itself when we are about to start systemd-resolved, to avoid deadlocks. Note | |||
Well it's sexier to use DynamicUser=1 ;-) I figure this is a discussion to have, but it's kinda independent of this specific PR. Moreover, now that that 5b5d826 has been merged we are basically back to static users for those three services anyway... |
The header file references strlen(), hence it should include string.h
One of those days we should rework this to use the UNIQ macros, but for now, an underscore should be enough.
This is an attempt to automatically detect and avoid certain kinds of NSS deadlocks as discussed in this thread: https://lists.freedesktop.org/archives/systemd-devel/2018-July/040975.html
|
Ahh, I think I got @lucab's comment now. Will shortly push a new version that explicitly says the value is the unit's name. This was missing in the original version by mistake. |
|
(just to clarify this: the new version i refer to in the comment is the one that is already here... for some reason github showed the message after the push, even though it happened the other way round) |
Triggered by this thread:
https://lists.freedesktop.org/archives/systemd-devel/2018-July/040975.html
plus some other tiny microfixlets