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.
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
Signed-off-by: NeilBrown <[email protected]>
---
configure.ac | 4 ++--
src/warmstart.c | 5 +----
2 files changed, 3 insertions(+), 6 deletions(-)
diff --git a/configure.ac b/configure.ac
index f84921eb27fb..fe7d0b068439 100644
--- a/configure.ac
+++ b/configure.ac
@@ -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=/run/rpcbind@:>@])
+ ,, [with_statedir=/run/rpcbind])
AC_SUBST([statedir], [$with_statedir])
AC_ARG_WITH([rpcuser],
diff --git a/src/warmstart.c b/src/warmstart.c
index 122a058b7954..3896027e62ba 100644
--- a/src/warmstart.c
+++ b/src/warmstart.c
@@ -48,10 +48,6 @@
#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
@@ -141,6 +137,7 @@ error:
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);
--
2.10.2
On 11 Nov 2016 14:36, NeilBrown wrote:
> 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.
-mike
On Sat, Nov 12 2016, Mike Frysinger wrote:
> [ Unknown signature status ]
> On 11 Nov 2016 14:36, NeilBrown wrote:
>> 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.
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.
https://en.wikipedia.org/wiki/Unix_filesystem#Conventional_directory_layout
only suggests /tmp as something that won't survive reboot.
Maybe we should continue to use the /tmp filesystem, but move to a
subdirectory:
/tmp/rpcbind
??
Thanks,
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.
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(-)
diff --git a/configure.ac b/configure.ac
index f84921eb27fb..df931c720f93 100644
--- a/configure.ac
+++ b/configure.ac
@@ -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],
diff --git a/src/rpcbind.c b/src/rpcbind.c
index 87ccdc27e4c9..8db8dfc17c27 100644
--- a/src/rpcbind.c
+++ b/src/rpcbind.c
@@ -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);
diff --git a/src/rpcbind.h b/src/rpcbind.h
index 74f9591ae720..5b1a9bb8651a 100644
--- a/src/rpcbind.h
+++ b/src/rpcbind.h
@@ -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);
diff --git a/src/warmstart.c b/src/warmstart.c
index 122a058b7954..3a6bcb5e34e1 100644
--- a/src/warmstart.c
+++ b/src/warmstart.c
@@ -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);
--
2.10.2
On 14 Nov 2016 10:09, NeilBrown wrote:
> On Sat, Nov 12 2016, Mike Frysinger wrote:
> > On 11 Nov 2016 14:36, NeilBrown wrote:
> >> 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.
>
> 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.
-mike
On 11/14/2016 02:12 PM, Mike Frysinger wrote:
> On 14 Nov 2016 10:09, NeilBrown wrote:
>> On Sat, Nov 12 2016, Mike Frysinger wrote:
>>> On 11 Nov 2016 14:36, NeilBrown wrote:
>>>> 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.
>>
>> 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
steved.
On 14 Nov 2016 14:26, Steve Dickson wrote:
> On 11/14/2016 02:12 PM, Mike Frysinger wrote:
> > On 14 Nov 2016 10:09, NeilBrown wrote:
> >> On Sat, Nov 12 2016, Mike Frysinger wrote:
> >>> On 11 Nov 2016 14:36, NeilBrown wrote:
> >>>> 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.
> >>
> >> 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
yes, but we already know that's not an acceptable default -- /run today
is Linux specific. the question i was responding to here is if there's
a portable location that is better than /tmp.
Linux distros already know that for many packages they need to pass
flags to get /run behavior. rpcbind is no different.
-mike
On 11/14/2016 02:05 AM, NeilBrown wrote:
> 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.
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?
steved.
>
> NeilBrown
>
>
> configure.ac | 4 ++--
> src/rpcbind.c | 5 +++++
> src/rpcbind.h | 1 +
> src/warmstart.c | 25 +++++++++++++++++++++----
> 4 files changed, 29 insertions(+), 6 deletions(-)
>
> diff --git a/configure.ac b/configure.ac
> index f84921eb27fb..df931c720f93 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -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],
> diff --git a/src/rpcbind.c b/src/rpcbind.c
> index 87ccdc27e4c9..8db8dfc17c27 100644
> --- a/src/rpcbind.c
> +++ b/src/rpcbind.c
> @@ -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);
> diff --git a/src/rpcbind.h b/src/rpcbind.h
> index 74f9591ae720..5b1a9bb8651a 100644
> --- a/src/rpcbind.h
> +++ b/src/rpcbind.h
> @@ -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);
>
> diff --git a/src/warmstart.c b/src/warmstart.c
> index 122a058b7954..3a6bcb5e34e1 100644
> --- a/src/warmstart.c
> +++ b/src/warmstart.c
> @@ -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);
On Wed, Nov 16 2016, Steve Dickson wrote:
> On 11/14/2016 02:05 AM, NeilBrown wrote:
>> 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.
> 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?
/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
On 11/15/2016 08:34 PM, NeilBrown wrote:
> On Wed, Nov 16 2016, Steve Dickson wrote:
>
>> On 11/14/2016 02:05 AM, NeilBrown wrote:
>>> 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.
>> 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?
> /var/run rather than /run seems to be a safer universal default.
> Linux distros can run ./configure --with-statedir=/run/rcpbind
Fair enough... I can roll with that.
>
> Otherwise, I think we are in agreement.
>
> You want I should respin with /tmp/rpcbind -> /var/run/rpcbind ??
Sure...
thanks!
steved.
>
> Thanks,
> NeilBrown