[PATCH 0/3] some nfs-utils patches.

[PATCH 0/3] some nfs-utils patches.

[NeilBrown]

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

[PATCH 1/3] mountd: Initialize logging early.

[NeilBrown]

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,

[PATCH 2/3] conffile: allow optional include files.

[NeilBrown]

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.
 

[PATCH 3/3] statd: take user-id from /var/lib/nfs/sm

[NeilBrown]

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

Re: [PATCH 0/3] some nfs-utils patches.

[Steve Dickson]

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
> 

Re: [PATCH 0/3] some nfs-utils patches.

[NeilBrown]

[-- 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 --]