Bug 253959
| Summary: | service(8) env call with LDAP makes boot take many hours | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | Base System | Reporter: | Brad Davis <brd> | ||||||
| Component: | bin | Assignee: | Kyle Evans <kevans> | ||||||
| Status: | Closed FIXED | ||||||||
| Severity: | Affects Only Me | CC: | andrew, hmizuno, re | ||||||
| Priority: | --- | Flags: | brd:
mfc-stable13?
kevans: mfc-stable12? kevans: mfc-stable11- |
||||||
| Version: | Unspecified | ||||||||
| Hardware: | Any | ||||||||
| OS: | Any | ||||||||
| Attachments: |
|
||||||||
|
Description
Brad Davis
2021-03-02 16:36:16 UTC
FWIW, I think that this change is a *good* thing, but wondering if there is some way to mitigate env(1) or PAM from being so.. brain dead? (In reply to Brad Davis from comment #1) I suspect that that would require some more extensive hacking-in, but I've CC'd Andrew to solicit his opinion. Maybe a flag to service to prevent adding `-L 0/daemon' ? That way when service(8) is called via devd it can be disabled? Granted that doesn't fix all of it, but does take away at least a good portion of the pain. Created attachment 222920 [details]
PoC
I created a quick patch to test out..
I think that is completely the wrong approach - let me check something Brad, can you show all the changes you made to your base system config in order to use pam_ldap? Created attachment 222931 [details]
Make service(8) environment more consistent with init(8).
Does this patch help at all?
I can't see any way for env to end up invoking pam, but with -L it does do getpw* functions that might go through nsswitch, which could be an issue. So this avoids doing that, and also fixes up an inconsistency between what service(8) was doing and what init(8) does.
(In reply to andrew from comment #7) IMO this is fairly low-impact and an obviously good idea, so I'm going to go ahead and commit it to get some appropriate soak time in given the stage of the release cycle we're at. A commit in branch main references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=55deb0a5f089c8a27cfc1666655b93881c2b47ae commit 55deb0a5f089c8a27cfc1666655b93881c2b47ae Author: Andrew Gierth <andrew@tao146.riddles.org.uk> AuthorDate: 2021-03-03 18:25:11 +0000 Commit: Kyle Evans <kevans@FreeBSD.org> CommitDate: 2021-03-03 18:25:11 +0000 service(8): use an environment more consistent with init(8) init(8) sets the "daemon" login class without specifying a pw entry (so no substitutions are done on the variables). service(8)'s use of env -L had the effect of specifying root's pw entry, with two effects: getpwnam and getpwuid are being called, which may not be entirely safe depending on what nsswitch is up to and what stage of boot we are at, and substitutions would have been done. Fix by teaching env(8) to allow -L -/classname to set the class environment with no pw entry at all specified, and use it in service(8). PR: 253959 usr.bin/env/env.1 | 7 ++++++- usr.bin/env/env.c | 25 ++++++++++++++++--------- usr.sbin/service/service.sh | 2 +- 3 files changed, 23 insertions(+), 11 deletions(-) This works, thank you! Starting a CLI argument with a dash seems kinda ugly.. A commit in branch stable/13 references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=872ec7e5b6f35d84745b49c02f58572632de22ed commit 872ec7e5b6f35d84745b49c02f58572632de22ed Author: Andrew Gierth <andrew@tao146.riddles.org.uk> AuthorDate: 2021-03-03 18:25:11 +0000 Commit: Kyle Evans <kevans@FreeBSD.org> CommitDate: 2021-03-07 21:39:30 +0000 service(8): use an environment more consistent with init(8) init(8) sets the "daemon" login class without specifying a pw entry (so no substitutions are done on the variables). service(8)'s use of env -L had the effect of specifying root's pw entry, with two effects: getpwnam and getpwuid are being called, which may not be entirely safe depending on what nsswitch is up to and what stage of boot we are at, and substitutions would have been done. Fix by teaching env(8) to allow -L -/classname to set the class environment with no pw entry at all specified, and use it in service(8). PR: 253959 (cherry picked from commit 55deb0a5f089c8a27cfc1666655b93881c2b47ae) (cherry picked from commit 0c1a5eaae83267365330437adb60f44e1a622a2b) usr.bin/env/env.1 | 7 ++++++- usr.bin/env/env.c | 25 ++++++++++++++++--------- usr.sbin/service/service.sh | 2 +- 3 files changed, 23 insertions(+), 11 deletions(-) A commit in branch releng/13.0 references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=cb370b19715b696cf6db4f7b357cf2e7f2e3adb7 commit cb370b19715b696cf6db4f7b357cf2e7f2e3adb7 Author: Andrew Gierth <andrew@tao146.riddles.org.uk> AuthorDate: 2021-03-03 18:25:11 +0000 Commit: Kyle Evans <kevans@FreeBSD.org> CommitDate: 2021-03-17 00:11:23 +0000 service(8): use an environment more consistent with init(8) init(8) sets the "daemon" login class without specifying a pw entry (so no substitutions are done on the variables). service(8)'s use of env -L had the effect of specifying root's pw entry, with two effects: getpwnam and getpwuid are being called, which may not be entirely safe depending on what nsswitch is up to and what stage of boot we are at, and substitutions would have been done. Fix by teaching env(8) to allow -L -/classname to set the class environment with no pw entry at all specified, and use it in service(8). PR: 253959 Approved by: re (gjb) (cherry picked from commit 55deb0a5f089c8a27cfc1666655b93881c2b47ae) (cherry picked from commit 0c1a5eaae83267365330437adb60f44e1a622a2b) (cherry picked from commit 872ec7e5b6f35d84745b49c02f58572632de22ed) usr.bin/env/env.1 | 7 ++++++- usr.bin/env/env.c | 25 ++++++++++++++++--------- usr.sbin/service/service.sh | 2 +- 3 files changed, 23 insertions(+), 11 deletions(-) In time for 13.0-rc3; thanks, folks. Seems like 12.3-RELEASE still has this Bug. https://cgit.freebsd.org/src/tree/usr.sbin/service/service.sh?h=release/12.3.0 https://cgit.freebsd.org/src/tree/usr.bin/env/env.c?h=release/12.3.0 On my machine, with the following nsswitch.conf (and LDAP also configured), booting the OS took long time to boot at starting devd. # cat /etc/nsswitch.conf # # nsswitch.conf(5) - name service switch configuration file # $FreeBSD: releng/12.3/lib/libc/net/nsswitch.conf 338729 2018-09-17 18:56:47Z brd $ # group: compat group_compat: ldap hosts: files dns netgroup: compat networks: files passwd: compat passwd_compat: ldap shells: files services: compat services_compat: nis protocols: files rpc: files Thank you for your consideration, in advance. |