Move default state-dir to /run/rpcbind

rpcbind can save state in a file to allow restart without forgetting about running services.

The default location is currently "/tmp" which is an over-used directory that isn't really suitable for system files.
The modern preferences would be a subdirectory of "/run", which can be selected with a ./configure option. That subdirectory would still need to be created by something.

[ Unknown signature status ]

Quoted 3 lines · NeilBrown

rpcbind can save state in a file to allow restart without forgetting about running services.

Quoted 5 lines

The default location is currently "/tmp" which is an over-used directory that isn't really suitable for system files.
The modern preferences would be a subdirectory of "/run", which can be selected with a ./configure option. That subdirectory would still need to be created by something.

the portable path is /var/cache instead of /run. i don't think libtirpc should be configuring itself to assume Linux by default.

rpcbind can save state in a file to allow restart without forgetting about running services.

The default location is currently "/tmp" which is not ideal for system files. It is particularly unpleasant to put simple files there rather than creating a directory to contain them.

On a modern Linux system it is preferable to use /run, and there it is even more consistent with practice to use a subdirectory.

This directory needs to be create one each boot, and while there are tools (e.g. systemd-tmpfiles) which can do that it is cleaner to keep rpcbind self-contained and have it create the directory.

So change the default location to /tmp/rpcbind, and create that directory. If a different user-id is used, we need to create and chown the directory before dropping privileges. We do this with care so avoid chowning the wrong thing by mistake.

Signed-off-by NeilBrown <[email protected]>

---

hi,
I realized that I hadn't allowed for the fact that rpcbind changes it's uid, and we need to mkdir and chown before that. I've also reverted the move to /run, but moved to /tmp/rpcbind instead. A subdirectory is a good idea, even in /tmp.

NeilBrown

 configure.ac    |  4 ++--
 src/rpcbind.c   |  5 +++++
 src/rpcbind.h   |  1 +
 src/warmstart.c | 25 +++++++++++++++++++++----
 4 files changed, 29 insertions(+), 6 deletions(-)

configure.ac +2 -2

index f84921eb27fb..df931c720f93 100644
@@ -22,8 +22,8 @@ AC_ARG_ENABLE([warmstarts],
 AM_CONDITIONAL(WARMSTART, test x$enable_warmstarts = xyes)
 
 AC_ARG_WITH([statedir],
-  AS_HELP_STRING([--with-statedir=ARG], [use ARG as state dir @<:@default=/tmp@:>@])
-  ,, [with_statedir=/tmp])
+  AS_HELP_STRING([--with-statedir=ARG], [use ARG as state dir @<:@default=/tmp/rpcbind@:>@])
+  ,, [with_statedir=/tmp/rpcbind])
 AC_SUBST([statedir], [$with_statedir])
 
 AC_ARG_WITH([rpcuser],

src/rpcbind.c +5 -0

index 87ccdc27e4c9..8db8dfc17c27 100644
@@ -263,6 +263,11 @@ main(int argc, char *argv[])
 			syslog(LOG_ERR, "cannot get uid of '%s': %m", id);
 			exit(1);
 		}
+#ifdef WARMSTART
+		if (warmstart) {
+			mkdir_warmstart(p->pw_uid);
+		}
+#endif
                 if (setgid(p->pw_gid) == -1) {
                         syslog(LOG_ERR, "setgid to '%s' (%d) failed: %m", id, p->pw_gid);
                         exit(1);

src/rpcbind.h +1 -0

index 74f9591ae720..5b1a9bb8651a 100644
@@ -129,6 +129,7 @@ int is_localroot(struct netbuf *);
 extern void pmap_service(struct svc_req *, SVCXPRT *);
 #endif
 
+void mkdir_warmstart(int uid);
 void write_warmstart(void);
 void read_warmstart(void);
 

src/warmstart.c +21 -4

index 122a058b7954..3a6bcb5e34e1 100644
@@ -45,19 +45,23 @@
 #include <syslog.h>
 #include <unistd.h>
 #include <errno.h>
+#include <fcntl.h>
 
 #include "rpcbind.h"
 
-#ifndef RPCBIND_STATEDIR
-#define RPCBIND_STATEDIR "/tmp"
-#endif
-
 /* These files keep the pmap_list and rpcb_list in XDR format */
 #define	RPCBFILE	RPCBIND_STATEDIR "/rpcbind.xdr"
 #ifdef PORTMAP
 #define	PMAPFILE	RPCBIND_STATEDIR "/portmap.xdr"
 #endif
 
+#ifndef O_DIRECTORY
+#define O_DIRECTORY 0
+#endif
+#ifndef O_NOFOLLOW
+#define O_NOFOLLOW 0
+#endif
+
 static bool_t write_struct(char *, xdrproc_t, void *);
 static bool_t read_struct(char *, xdrproc_t, void *);
 

@@ -139,8 +143,21 @@ error:
 }
 
 void
+mkdir_warmstart(int uid)
+{
+	if (mkdir(RPCBIND_STATEDIR, 0770) == 0) {
+		int fd = open(RPCBIND_STATEDIR, O_RDONLY | O_DIRECTORY | O_NOFOLLOW);
+		if (fd >= 0) {
+			fchown(fd, uid, -1);
+			close(fd);
+		}
+	}
+}
+
+void
 write_warmstart()
 {
+	(void) mkdir(RPCBIND_STATEDIR, 0770);
 	(void) write_struct(RPCBFILE, (xdrproc_t)xdr_rpcblist_ptr, &list_rbl);
 #ifdef PORTMAP
 	(void) write_struct(PMAPFILE, (xdrproc_t)xdr_pmaplist_ptr, &list_pml);

Quoted 20 lines · NeilBrown

rpcbind can save state in a file to allow restart without forgetting about running services.

The default location is currently "/tmp" which is not ideal for system files. It is particularly unpleasant to put simple files there rather than creating a directory to contain them.

On a modern Linux system it is preferable to use /run, and there it is even more consistent with practice to use a subdirectory.

This directory needs to be create one each boot, and while there are tools (e.g. systemd-tmpfiles) which can do that it is cleaner to keep rpcbind self-contained and have it create the directory.

So change the default location to /tmp/rpcbind, and create that directory. If a different user-id is used, we need to create and chown the directory before dropping privileges. We do this with care so avoid chowning the wrong thing by mistake.

Quoted 8 lines

Signed-off-by NeilBrown <[email protected]>

---

hi,
I realized that I hadn't allowed for the fact that rpcbind changes it's uid, and we need to mkdir and chown before that. I've also reverted the move to /run, but moved to /tmp/rpcbind instead. A subdirectory is a good idea, even in /tmp.

I'm beginning to think put these files into a directory call /tmp/rpcbind is not a good idea... Because if something in /tmp is called rpcbind (like a debugging binary ;-) ) the mkdirs will silently fail which is not good.

Here is what I would like to do.

Move the directory into /run then create the /run/rpcbind when it does not exist... I think that should play nicely in both the systemd worlds and non-systemd worlds

Thoughts?

Quoted 35 lines · Steve Dickson

Quoted 20 lines · NeilBrown

rpcbind can save state in a file to allow restart without forgetting about running services.

The default location is currently "/tmp" which is not ideal for system files. It is particularly unpleasant to put simple files there rather than creating a directory to contain them.

On a modern Linux system it is preferable to use /run, and there it is even more consistent with practice to use a subdirectory.

This directory needs to be create one each boot, and while there are tools (e.g. systemd-tmpfiles) which can do that it is cleaner to keep rpcbind self-contained and have it create the directory.

So change the default location to /tmp/rpcbind, and create that directory. If a different user-id is used, we need to create and chown the directory before dropping privileges. We do this with care so avoid chowning the wrong thing by mistake.

Quoted 8 lines

Signed-off-by NeilBrown <[email protected]>

---

hi,
I realized that I hadn't allowed for the fact that rpcbind changes it's uid, and we need to mkdir and chown before that. I've also reverted the move to /run, but moved to /tmp/rpcbind instead. A subdirectory is a good idea, even in /tmp.

I'm beginning to think put these files into a directory call /tmp/rpcbind is not a good idea... Because if something in /tmp is called rpcbind (like a debugging binary ;-) ) the mkdirs will silently fail which is not good.

Here is what I would like to do.

Quoted 5 lines

Move the directory into /run then create the /run/rpcbind when it does not exist... I think that should play nicely in both the systemd worlds and non-systemd worlds

Thoughts?

/var/run rather than /run seems to be a safer universal default. Linux distros can run ./configure --with-statedir=/run/rcpbind

Otherwise, I think we are in agreement.

You want I should respin with /tmp/rpcbind -> /var/run/rpcbind ??

Thanks,
NeilBrown

Quoted 4 lines · Mike Frysinger

Quoted 3 lines · NeilBrown

rpcbind can save state in a file to allow restart without forgetting about running services.

Quoted 8 lines

Quoted 5 lines

The default location is currently "/tmp" which is an over-used directory that isn't really suitable for system files.
The modern preferences would be a subdirectory of "/run", which can be selected with a ./configure option. That subdirectory would still need to be created by something.

the portable path is /var/cache instead of /run. i don't think libtirpc should be configuring itself to assume Linux by default.

In principle I agree. But is /var/cache really a good choice? We don't want the state files to persist over a reboot, and I strongly suspect that /var/cache is designed to do exactly that.

Are there agree standards that are broader than Linux that we can look to?
FHS defines /var/run (or even /run) but I suspect it is linux-only.

Quoted 12 lines · NeilBrown

Quoted 11 lines · Mike Frysinger

Quoted 8 lines · NeilBrown

rpcbind can save state in a file to allow restart without forgetting about running services.

The default location is currently "/tmp" which is an over-used directory that isn't really suitable for system files.
The modern preferences would be a subdirectory of "/run", which can be selected with a ./configure option. That subdirectory would still need to be created by something.

the portable path is /var/cache instead of /run. i don't think libtirpc should be configuring itself to assume Linux by default.

Quoted 7 lines

In principle I agree. But is /var/cache really a good choice? We don't want the state files to persist over a reboot, and I strongly suspect that /var/cache is designed to do exactly that.

Are there agree standards that are broader than Linux that we can look to?
FHS defines /var/run (or even /run) but I suspect it is linux-only.

/var/run should work across systems i believe. at least BSD's support it.

Quoted 17 lines · Mike Frysinger

Quoted 16 lines · NeilBrown

Quoted 11 lines · Mike Frysinger

Quoted 8 lines · NeilBrown

rpcbind can save state in a file to allow restart without forgetting about running services.

The default location is currently "/tmp" which is an over-used directory that isn't really suitable for system files.
The modern preferences would be a subdirectory of "/run", which can be selected with a ./configure option. That subdirectory would still need to be created by something.

the portable path is /var/cache instead of /run. i don't think libtirpc should be configuring itself to assume Linux by default.

In principle I agree. But is /var/cache really a good choice? We don't want the state files to persist over a reboot, and I strongly suspect that /var/cache is designed to do exactly that.

Quoted 4 lines

Quoted 3 lines

Are there agree standards that are broader than Linux that we can look to?
FHS defines /var/run (or even /run) but I suspect it is linux-only.

/var/run should work across systems i believe. at least BSD's support it.

In the Red Hat distos /var/run is a symbolic link to /run and the systemd folks have asked us to use /run instead of /var/run

Quoted 9 lines · NeilBrown

rpcbind can save state in a file to allow restart without forgetting about running services.

The default location is currently "/tmp" which is an over-used directory that isn't really suitable for system files.
The modern preferences would be a subdirectory of "/run", which can be selected with a ./configure option. That subdirectory would still need to be created by something.

Quoted 6 lines

It is trivial for rpcbind to create the directory itself, and harmless to try if it already exists, so:
- add a "mkdir" when saving state data
- change the default to /run/rpcbind (currently used by Debian)
- remove the default settign in the code, just use the one in configure.ac

I'm all for move the warmstart directory to /run but why don't we have systemd create the directory via a tmpfiles.d( config file... some like

#Type Path Mode UID GID Age Argument
D /run/rpcbind 0700 rpc rpc - -

The only thing I'm not sure about is how it would get installed... I guess it would some type of Makefile.ac entry??

Quoted 19 lines · Steve Dickson

Quoted 9 lines · NeilBrown

rpcbind can save state in a file to allow restart without forgetting about running services.

The default location is currently "/tmp" which is an over-used directory that isn't really suitable for system files.
The modern preferences would be a subdirectory of "/run", which can be selected with a ./configure option. That subdirectory would still need to be created by something.

Quoted 6 lines

It is trivial for rpcbind to create the directory itself, and harmless to try if it already exists, so:
- add a "mkdir" when saving state data
- change the default to /run/rpcbind (currently used by Debian)
- remove the default settign in the code, just use the one in configure.ac

I'm all for move the warmstart directory to /run but why don't we have systemd create the directory via a tmpfiles.d( config file... some like

Quoted 5 lines

#Type Path Mode UID GID Age Argument
D /run/rpcbind 0700 rpc rpc - -

The only thing I'm not sure about is how it would get installed... I guess it would some type of Makefile.ac entry??

Because not everyone uses systemd?

Someone at SUSE recently moved the state directory to /run/rpcbind and used tmpfiles.d exactly as you describe to create /run/rpcbind. Then found they needed to do something extra and different for dracut. I haven't looked into why, but it is presumably because while dracut does use systemd, it doesn't use it quite the same way that normal system boot uses it.

I've found this with configuring mdadm too. I got the udev and systemd configuration just right, and then found that while dracut uses udev and systemd, I still need to add extra stuff to dracut. And then there was another boot environment which used udev but not systemd, so it needed different tweaking.

So I think we are less likely to run into strange problems if we just get rpcbind to create its own directory. Certainly rely on systemd to do the things that systemd does best (like start the service), but don't rely on it to do things we can easily do ourselves.

Thanks,
NeilBrown

Quoted 21 lines · NeilBrown

Quoted 19 lines · Steve Dickson

Quoted 9 lines · NeilBrown

rpcbind can save state in a file to allow restart without forgetting about running services.

The default location is currently "/tmp" which is an over-used directory that isn't really suitable for system files.
The modern preferences would be a subdirectory of "/run", which can be selected with a ./configure option. That subdirectory would still need to be created by something.

Quoted 6 lines

It is trivial for rpcbind to create the directory itself, and harmless to try if it already exists, so:
- add a "mkdir" when saving state data
- change the default to /run/rpcbind (currently used by Debian)
- remove the default settign in the code, just use the one in configure.ac

I'm all for move the warmstart directory to /run but why don't we have systemd create the directory via a tmpfiles.d( config file... some like

Quoted 6 lines

Quoted 5 lines

#Type Path Mode UID GID Age Argument
D /run/rpcbind 0700 rpc rpc - -

The only thing I'm not sure about is how it would get installed... I guess it would some type of Makefile.ac entry??

Because not everyone uses systemd?

Yeah... I figured as much.

Quoted 6 lines

Someone at SUSE recently moved the state directory to /run/rpcbind and used tmpfiles.d exactly as you describe to create /run/rpcbind. Then found they needed to do something extra and different for dracut. I haven't looked into why, but it is presumably because while dracut does use systemd, it doesn't use it quite the same way that normal system boot uses it.

I didn't have this problem when I move rpcbind in RHEL7 to use /run/rpcbind via tmpfiles.d The biggest problem I had was migrating the warmstart files to the new place. Having /run/rpcbind exist after boot made that much easier.

Quoted 6 lines

I've found this with configuring mdadm too. I got the udev and systemd configuration just right, and then found that while dracut uses udev and systemd, I still need to add extra stuff to dracut. And then there was another boot environment which used udev but not systemd, so it needed different tweaking.

Quoted 4 lines

So I think we are less likely to run into strange problems if we just get rpcbind to create its own directory. Certainly rely on systemd to do the things that systemd does best (like start the service), but don't rely on it to do things we can easily do ourselves.

Another potential problem is SElinux... Its never a fan of daemon creating things on the fly... but I guess that is my problem 8-)