These free are largely unrelated. The only connection is that without the second, I get warnings because my /etc/nfs.conf includes /etc/nfs.conf.local - just in case. Then, without the first patch, the open fds get confused and rpc.mountd doesn't listen on all /proc/net/rpc/*/channel properly and nfs doesn't work. Thanks, NeilBrown --- NeilBrown (3): mountd: Initialize logging early. conffile: allow optional include files. statd: take user-id from /var/lib/nfs/sm support/nfs/conffile.c | 13 ++++++++++--- support/nsm/file.c | 16 +++++----------- systemd/nfs.conf.man | 3 +++ utils/mountd/mountd.c | 9 +++------ utils/statd/sm-notify.man | 10 +++++++++- utils/statd/statd.man | 10 +++++++++- 6 files changed, 39 insertions(+), 22 deletions(-) -- Signature
Reading the config file can generate log messages, so we should initialize logging before reading the config file. If any log message are generated, syslog will leave a file descriptor open (a socket), so calling closeall(3) after this can cause problem. Before this we initialize login we don't know if Foreground (-F) has been selected, so closeall() cannot be conditional on that. closeall() isn't needed - daemon are almost always run from a management daemon like systemd, and they are given a clean environment. It is really best if they just take what they are given. So remove the closeall() call. Signed-off-by: NeilBrown <neilb@suse.de> --- utils/mountd/mountd.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/utils/mountd/mountd.c b/utils/mountd/mountd.c index 33571ecbd401..5a12d0bcd19e 100644 --- a/utils/mountd/mountd.c +++ b/utils/mountd/mountd.c @@ -681,6 +681,9 @@ main(int argc, char **argv) else progname = argv[0]; + /* Initialize logging. */ + xlog_open(progname); + conf_init_file(NFS_CONFFILE); xlog_from_conffile("mountd"); manage_gids = conf_get_bool("mountd", "manage-gids", manage_gids); @@ -820,9 +823,7 @@ main(int argc, char **argv) } } } - /* Initialize logging. */ if (!foreground) xlog_stderr(0); - xlog_open(progname); sa.sa_handler = SIG_IGN; sa.sa_flags = 0; @@ -834,10 +835,6 @@ main(int argc, char **argv) /* WARNING: the following works on Linux and SysV, but not BSD! */ sigaction(SIGCHLD, &sa, NULL); - /* Daemons should close all extra filehandles ... *before* RPC init. */ - if (!foreground) - closeall(3); - unregister_services(); if (version2()) { listeners += nfs_svc_create("mountd", MOUNTPROG,
If nfs.conf contains, for example include = /etc/nfs.conf.local and /etc/nfs.conf.local doesn't exist, then a warning is given. Sometimes it is useful to have an optional include file which is included if present, but for which an absence doesn't give a warning. Systemd has a convention that a hyphen at the start of an include file name marks it as optional, so add this convention to nfs-utils. So include = -/etc/nfs.conf.local will not give a warning if the file doesn't exist. Signed-off-by: NeilBrown <neilb@suse.de> --- support/nfs/conffile.c | 13 ++++++++++--- systemd/nfs.conf.man | 3 +++ 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/support/nfs/conffile.c b/support/nfs/conffile.c index 6ba8a35ce7c6..d55bfe10120a 100644 --- a/support/nfs/conffile.c +++ b/support/nfs/conffile.c @@ -412,11 +412,18 @@ conf_parse_line(int trans, char *line, const char *filename, int lineno, char ** if (strcasecmp(line, "include")==0) { /* load and parse subordinate config files */ + _Bool optional = false; + + if (val && *val == '-') { + optional = true; + val++; + } + relpath = relative_path(filename, val); if (relpath == NULL) { - xlog_warn("config error at %s:%d: " - "error loading included config", - filename, lineno); + if (!optional) + xlog_warn("config error at %s:%d: error loading included config", + filename, lineno); return; } diff --git a/systemd/nfs.conf.man b/systemd/nfs.conf.man index d375bcc1d5a7..3f1c7261991d 100644 --- a/systemd/nfs.conf.man +++ b/systemd/nfs.conf.man @@ -65,6 +65,9 @@ section headers, then new sections will be created just as if the included file appeared in place of the .B include line. +If the file name starts with a hyphen then that is stripped off +before the file is opened, and if file doesn't exist no warning is +given. Normally a non-existent include file generates a warning. .PP Lookup of section and value names is case-insensitive.
Having /var/lib/nfs writeable by statd is not ideal as there are files in there that statd doesn't need to access. After dropping privs, statd and sm-notify only need to access files in the directories sm and sm.bak. So take the uid for these deamons from 'sm'. Signed-off-by: NeilBrown <neilb@suse.de> --- support/nsm/file.c | 16 +++++----------- utils/statd/sm-notify.man | 10 +++++++++- utils/statd/statd.man | 10 +++++++++- 3 files changed, 23 insertions(+), 13 deletions(-) diff --git a/support/nsm/file.c b/support/nsm/file.c index 0b66f123165e..f5b448015751 100644 --- a/support/nsm/file.c +++ b/support/nsm/file.c @@ -388,23 +388,17 @@ nsm_drop_privileges(const int pidfd) (void)umask(S_IRWXO); - /* - * XXX: If we can't stat dirname, or if dirname is owned by - * root, we should use "statduser" instead, which is set up - * by configure.ac. Nothing in nfs-utils seems to use - * "statduser," though. - */ - if (lstat(nsm_base_dirname, &st) == -1) { - xlog(L_ERROR, "Failed to stat %s: %m", nsm_base_dirname); - return false; - } - if (chdir(nsm_base_dirname) == -1) { xlog(L_ERROR, "Failed to change working directory to %s: %m", nsm_base_dirname); return false; } + if (lstat(NSM_MONITOR_DIR, &st) == -1) { + xlog(L_ERROR, "Failed to stat %s/%s: %m", nsm_base_dirname, NSM_MONITOR_DIR); + return false; + } + if (!prune_bounding_set()) return false; diff --git a/utils/statd/sm-notify.man b/utils/statd/sm-notify.man index cfe1e4b1dac8..addf5d3c028e 100644 --- a/utils/statd/sm-notify.man +++ b/utils/statd/sm-notify.man @@ -190,7 +190,15 @@ by default. After starting, .B sm-notify attempts to set its effective UID and GID to the owner -and group of this directory. +and group of the subdirectory +.B sm +of this directory. After changing the effective ids, +.B sm-notify +only needs to access files in +.B sm +and +.B sm.bak +within the state-directory-path. .TP .BI -v " ipaddr " | " hostname Specifies the network address from which to send reboot notifications, diff --git a/utils/statd/statd.man b/utils/statd/statd.man index 71d58461b5ea..6222701e38a8 100644 --- a/utils/statd/statd.man +++ b/utils/statd/statd.man @@ -259,7 +259,15 @@ by default. After starting, .B rpc.statd attempts to set its effective UID and GID to the owner -and group of this directory. +and group of the subdirectory +.B sm +of this directory. After changing the effective ids, +.B rpc.statd +only needs to access files in +.B sm +and +.B sm.bak +within the state-directory-path. .TP .BR -v ", " -V ", " --version Causes
On 9/23/19 12:26 AM, NeilBrown wrote: > These free are largely unrelated. > The only connection is that without the second, I get > warnings because my /etc/nfs.conf includes /etc/nfs.conf.local - just > in case. > Then, without the first patch, the open fds get confused and > rpc.mountd doesn't listen on all /proc/net/rpc/*/channel > properly and nfs doesn't work. > > Thanks, > NeilBrown > > --- > > NeilBrown (3): > mountd: Initialize logging early. > conffile: allow optional include files. > statd: take user-id from /var/lib/nfs/sm Committed... steved. > > > support/nfs/conffile.c | 13 ++++++++++--- > support/nsm/file.c | 16 +++++----------- > systemd/nfs.conf.man | 3 +++ > utils/mountd/mountd.c | 9 +++------ > utils/statd/sm-notify.man | 10 +++++++++- > utils/statd/statd.man | 10 +++++++++- > 6 files changed, 39 insertions(+), 22 deletions(-) > > -- > Signature >
[-- Attachment #1: Type: text/plain, Size: 1106 bytes --] On Mon, Oct 14 2019, Steve Dickson wrote: > On 9/23/19 12:26 AM, NeilBrown wrote: >> These free are largely unrelated. >> The only connection is that without the second, I get >> warnings because my /etc/nfs.conf includes /etc/nfs.conf.local - just >> in case. >> Then, without the first patch, the open fds get confused and >> rpc.mountd doesn't listen on all /proc/net/rpc/*/channel >> properly and nfs doesn't work. >> >> Thanks, >> NeilBrown >> >> --- >> >> NeilBrown (3): >> mountd: Initialize logging early. >> conffile: allow optional include files. >> statd: take user-id from /var/lib/nfs/sm > Committed... Thanks a lot Steve! NeilBrown > > steved. >> >> >> support/nfs/conffile.c | 13 ++++++++++--- >> support/nsm/file.c | 16 +++++----------- >> systemd/nfs.conf.man | 3 +++ >> utils/mountd/mountd.c | 9 +++------ >> utils/statd/sm-notify.man | 10 +++++++++- >> utils/statd/statd.man | 10 +++++++++- >> 6 files changed, 39 insertions(+), 22 deletions(-) >> >> -- >> Signature >> [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 832 bytes --]