pidfile_open() usage in "mount"

[Konstantin Belousov]

On Tue, Aug 25, 2020 at 10:06:22PM -0400, J David wrote:
> It looks like the "mount" program creates /var/run/mountd.pid every
> time it runs, if mountd is not itself running.
> 
> This code appears in sbin/mount/mount.c:
> 
> static void
> restart_mountd(void)
> {
>     struct pidfh *pfh;
>     pid_t mountdpid;
> 
>     mountdpid = 0;
>     pfh = pidfile_open(_PATH_MOUNTDPID, 0600, &mountdpid);
>     if (pfh != NULL) {
>         /* Mountd is not running. */
>         pidfile_remove(pfh);
>         return;
>     }
> 
> pidfile_open(3) *creates* /var/run/mountd.pid if it doesn't already
> exist, hence the need to delete it if the call actually succeeds.
> This leads to a race condition when multiple mounts occur at the same
> time.  That case is handled later in the code:
> 
>     /*
>      * Refuse to send broadcast or group signals, this has
>      * happened due to the bugs in pidfile(3).
>      */
>     if (mountdpid <= 0) {
>         warnx("mountd pid %d, refusing to send SIGHUP", mountdpid);
>         return;
>     }
> 
> "mount" is not "mountd."  It seems inappropriate for it to, under any
> circumstances, create mountd's pid file.  The multiple workarounds for
> the problems that causes don't seem like the optimal approach.
> 
> This is something I'd be willing to open a bug and submit a patch for,
> but so as not to do work that stands no chance of being accepted, I'd
> like to understand first if the preferred approach would be to change
> mount.c not to use pidfile library calls at all, or if it would be
> better to add a function to the pidfile library similar to
> pidfile_open() designed for "consumer" use that would never create the
> file, leaving pidfile_open() for "producer" use?
I think that a new libutil/pidfile.c function, to open only existing
pid file, is the right approach.

It is possible that both pidfile_open() and the new function would share
some significant amount of code.