Skip site navigation (1)Skip section navigation (2)
Date:      Sun, 23 Jun 2002 11:56:03 -0500
From:      "Matthew D. Fuller" <fullermd@over-yonder.net>
To:        "Geoffrey C. Speicher" <geoff@sea-incorporated.com>
Cc:        freebsd-hackers@freebsd.org, Matt Simerson <freebsd@blockads.com>, Paul Herman <pherman@frenchfries.net>, Oliver Fromme <olli@secnetix.de>
Subject:   Re: Locking the passwd subsystem (was Re: bug in pw, -STABLE [patch])
Message-ID:  <20020623165602.GF81018@over-yonder.net>
In-Reply-To: <20020623105835.E29860-300000@sea-incorporated.com>
References:  <20020623135441.GC81018@over-yonder.net> <20020623105835.E29860-300000@sea-incorporated.com>

next in thread | previous in thread | raw e-mail | index | archive | help

--aM3YZ0Iwxop3KEKx
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

On Sun, Jun 23, 2002 at 11:17:35AM -0400 I heard the voice of
Geoffrey C. Speicher, and lo! it spake thus:
>
> 5-minute window.  Still unliklely, but not quite as unlikely as the
> non-daemon scenario.

And it all goes out when window when kern.randompid=1, so it's kinda
moot.


> Looks good from here.  I've attached some minor patches (also relative to
> src/) to pid_util.{3,c}.  The first corrects some minor typos in the man
> page and the second is to make pid_begin() work as advertised when read()
> fails (set errno and return -1 rather than return errno).

Yes, I have a brain.  No, really I do.  I just don't have enough coffee
:)


> (I haven't tested any of this yet either.)

I just slapped together a few quick test programs.  Here's an updated
patch, incorporating your fixes, as well as a bugfix and an almost-bugfix
my testing showed up (hint: just because you ftruncate() to 0-length,
doesn't mean your offset becomes 0 too, dangit), and eliminated an extra
variable.


-- 
Matthew Fuller     (MF4839)   |  fullermd@over-yonder.net
Systems/Network Administrator |  http://www.over-yonder.net/~fullermd/

"The only reason I'm burning my candle at both ends, is because I
      haven't figured out how to light the middle yet"

--aM3YZ0Iwxop3KEKx
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename=diffs

Index: lib/libutil/Makefile
===================================================================
RCS file: /usr/cvs/src/lib/libutil/Makefile,v
retrieving revision 1.46
diff -u -r1.46 Makefile
--- lib/libutil/Makefile	8 May 2002 00:50:07 -0000	1.46
+++ lib/libutil/Makefile	23 Jun 2002 13:31:01 -0000
@@ -8,7 +8,7 @@
 CFLAGS+=-DINET6
 SRCS=	_secure_path.c auth.c fparseln.c login.c login_auth.c \
 	login_cap.c login_class.c login_crypt.c login_ok.c login_times.c \
-	login_tty.c logout.c logwtmp.c property.c pty.c \
+	login_tty.c logout.c logwtmp.c pid_util.c property.c pty.c \
 	pw_util.c realhostname.c stub.c \
 	trimdomain.c uucplock.c
 INCS=	libutil.h login_cap.h
@@ -16,7 +16,7 @@
 MAN+=	login.3 login_auth.3 login_tty.3 logout.3 logwtmp.3 pty.3 \
 	login_cap.3 login_class.3 login_times.3 login_ok.3 \
 	_secure_path.3 uucplock.3 property.3 auth.3 realhostname.3 \
-	realhostname_sa.3 trimdomain.3 fparseln.3
+	realhostname_sa.3 trimdomain.3 fparseln.3 pid_util.3
 MAN+=	login.conf.5 auth.conf.5
 MLINKS+= property.3 properties_read.3  property.3 properties_free.3
 MLINKS+= property.3 property_find.3
@@ -39,5 +39,6 @@
 MLINKS+=login_auth.3 auth_checknologin.3 login_auth.3 auth_cat.3
 MLINKS+=uucplock.3 uu_lock.3 uucplock.3 uu_lock_txfr.3 \
 	uucplock.3 uu_unlock.3 uucplock.3 uu_lockerr.3
+MLINKS+=pid_util.3 pid_begin.3 pid_end.3
 
 .include <bsd.lib.mk>
Index: lib/libutil/libutil.h
===================================================================
RCS file: /usr/cvs/src/lib/libutil/libutil.h,v
retrieving revision 1.37
diff -u -r1.37 libutil.h
--- lib/libutil/libutil.h	8 May 2002 00:50:07 -0000	1.37
+++ lib/libutil/libutil.h	23 Jun 2002 12:35:30 -0000
@@ -82,6 +82,8 @@
 struct sockaddr;
 int	realhostname_sa(char *host, size_t hsize, struct sockaddr *addr,
 			     int addrlen);
+int pid_begin(const char *pidfile, mode_t mode, int flags);
+int pid_end(const char *pidfile);
 #ifdef _STDIO_H_	/* avoid adding new includes */
 char   *fparseln(FILE *, size_t *, size_t *, const char[3], int);
 #endif
@@ -128,5 +130,8 @@
 /* pw_scan() */
 #define PWSCAN_MASTER		0x01
 #define PWSCAN_WARN		0x02
+
+/* pid_begin() */
+#define PID_NOBLOCK			0x01
 
 #endif /* !_LIBUTIL_H_ */
Index: lib/libutil/pid_util.c
===================================================================
--- /dev/null	Sun Jun 23 11:44:00 2002
+++ lib/libutil/pid_util.c	Sun Jun 23 11:50:59 2002
@@ -0,0 +1,178 @@
+/*-
+ * Copyright (c) 2002 Matthew D. Fuller <fullermd@over-yonder.net>
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in the
+ *    documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE AUTHORS AND CONTRIBUTORS ``AS IS'' AND
+ * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED.  IN NO EVENT SHALL THE AUTHORS OR CONTRIBUTORS BE LIABLE
+ * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
+ * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
+ * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
+ * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
+ * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
+ * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+ * SUCH DAMAGE.
+ */
+
+#ifndef lint
+static const char rcsid[] =
+  "$FreeBSD$";
+#endif /* not lint */
+
+/*
+ * These functions are for maintenance of locking PID files
+ */
+
+#include <sys/param.h>
+#include <sys/proc.h>
+#include <sys/uio.h>
+
+#include <errno.h>
+#include <fcntl.h>
+#include <signal.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <unistd.h>
+
+#include <libutil.h>
+
+
+#define PID_LEN	6	/* Currently only need 5 (0-99,999) */
+
+/*
+ * pid_begin: Open a PID file and write PID into it.  If one already
+ * exists, try and track down the process which opened it; if it can't be
+ * found, proceed as if the file weren't even there.
+ */
+int
+pid_begin(const char *pidfile, mode_t mode, int flags)
+{
+	int procdead=0;
+	int lockfd;
+	int holding=0;
+	ssize_t readlen;
+	pid_t masterpid;
+	char pidstr[PID_LEN+1];
+
+start:
+	if( (lockfd=open(pidfile, O_RDWR | O_CREAT | O_EXCL | O_EXLOCK,
+			mode)) < 0 )
+	{
+		if(errno!=EEXIST)
+			return(-1); /* Preserve errno */
+
+		/* Open and find PID */
+		if( (lockfd=open(pidfile, O_RDWR | O_EXLOCK, mode)) < 0 )
+		{
+			if(errno==ENOENT || errno==EWOULDBLOCK)
+			{
+				/*
+				 * This could use some thought.  We're going to
+				 * tight-loop if someone else has the file locked
+				 * (EWOULDBLOCK).  In theory, any locks should be
+				 * quite short-lived, so I THINK this will be fine.
+				 */
+				goto start;
+			}
+			else
+				return(-1);/* Preserve errno */
+		}
+
+		if( (readlen=read(lockfd, pidstr, PID_LEN)) < 0 )
+		{
+			holding==errno;
+			close(lockfd);
+			errno=holding;
+			return(-1);
+		}
+
+		pidstr[readlen]='\0';
+		masterpid = (pid_t) atoi(pidstr);
+		if(masterpid<1) /* This shouldn't happen */
+		{
+			/* This is really a NFS error, but we'll abuse it */
+			errno=ECANCELED;
+			return(-1);
+		}
+
+		/*
+		 * There's a number of ways we could approach this.  We could
+		 * stat /proc/$PID, but that would fail if /proc wasn't
+		 * mounted.  We could use libkvm, but that would require
+		 * linking libutil with it, as well as being setgid kmem on
+		 * the app (which we can't really count on).  We could
+		 * fork/exec ps(1), but that's a bit ugly.
+		 * I'm going to do it in a rather ugly way.  SIGWINCH is
+		 * fairly harmless in the cases where it's used, and is
+		 * ignored by default.  These functions are aimed at daemons,
+		 * which will generally have no use for it, and thus not
+		 * change it from the default discard.
+		 * Note that zombie'd processes count as 'nonexistent' to
+		 * kill(2).
+		 */
+		if( kill(masterpid, SIGWINCH) < 0 )
+			if(errno==ESRCH)
+				procdead=1;
+			else
+				procdead=0;
+		else
+			procdead=0; /* kill(2) succeeded */
+
+		if(procdead==0)
+		{
+			/* Old locker is still alive */
+			close(lockfd);
+			if( (flags & PID_NOBLOCK) )
+			{
+				errno=EWOULDBLOCK;
+				return(-1);
+			}
+			else
+			{
+				sleep(1); /* Arbitrary */
+				goto start;
+			}
+		}
+		/* Old lockholder is dead: fallthru */
+	}
+
+	/*
+	 * If we get here, we either ARE the O_EXCL opener, or the process
+	 * listed in the file is dead and we're taking it over.
+	 */
+	ftruncate(lockfd, 0);
+	lseek(lockfd, 0, SEEK_SET);
+	write( lockfd, pidstr, sprintf(pidstr, "%d", getpid()) );
+
+	close(lockfd);
+	return(0);
+}
+
+
+/*
+ * pid_end: Clean up a lock we hold.
+ */
+int
+pid_end(const char *pidfile)
+{
+	/*
+	 * We SHOULDN'T need to aquire a lock on the file, since the only
+	 * time anybody else should be writing into it would be if we're
+	 * dead, and thus we wouldn't be in this function.  If they're just
+	 * reading it, unlink()'ing it out from under them won't do any
+	 * damage.  If we're still alive, pid_begin() should close() the file
+	 * and attempt to re-open() for the next attempt, so this should work
+	 * nice and cleanly.
+	 */
+	return(unlink(pidfile));
+}
Index: lib/libutil/pid_util.3
===================================================================
--- /dev/null	Sun Jun 23 11:44:00 2002
+++ lib/libutil/pid_util.3	Sun Jun 23 11:51:39 2002
@@ -0,0 +1,106 @@
+.\" Copyright (c) 2002 Matthew D. Fuller <fullermd@over-yonder.net>
+.\" All rights reserved.
+.\"
+.\" Redistribution and use in source and binary forms, with or without
+.\" modification, are permitted provided that the following conditions
+.\" are met:
+.\" 1. Redistributions of source code must retain the above copyright
+.\"    notice, this list of conditions and the following disclaimer.
+.\" 2. Redistributions in binary form must reproduce the above copyright
+.\"    notice, this list of conditions and the following disclaimer in the
+.\"    documentation and/or other materials provided with the distribution.
+.\"
+.\" THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'' AND
+.\" ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+.\" IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+.\" ARE DISCLAIMED.  IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE LIABLE
+.\" FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
+.\" DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
+.\" OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
+.\" HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
+.\" LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
+.\" OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+.\" SUCH DAMAGE.
+.\"
+.\" $FreeBSD$
+.\"
+.Dd June 23, 2002
+.Os
+.Dt PID_UTIL 3
+.Sh NAME
+.Nm pid_begin ,
+.Nm pid_end
+.Nd "handle locking and unlocking of PID files"
+.Sh LIBRARY
+.Lb libutil
+.Sh SYNOPSIS
+.In sys/types.h
+.In libutil.h
+.Ft int
+.Fn pid_begin "const char *pidfile" "mode_t mode" "int flags"
+.Ft int
+.Fn pid_end "const char *pidfile"
+.Sh DESCRIPTION
+The function
+.Fn pid_begin
+attempts to open the file referenced by
+.Fa pidfile
+and write the process ID to it.
+If the file already exists,
+.Fn pid_begin
+will determine if the process listed in
+.Fa pidfile
+still exists.
+If the process is still alive,
+.Fn pid_begin
+will block and continue to try aquiring the lock indefinitely, unless
+.Dv PID_NOBLOCK
+is set in the
+.Fa flags ,
+in which case it will return an error.
+If the old process is dead, or the file doesn't exist,
+.Fn pid_begin
+will put its own process ID in the file and continue on its merry way.
+.Pp
+.Fn pid_end
+removes the file referenced by
+.Fa pidfile .
+Note that there is currently no protection afforded here that the process
+calling
+.Fn pid_end
+is actually the process that opened the PID file in the first place.
+.Sh RETURN VALUES
+If successful,
+.Fn pid_begin
+and
+.Fn pid_end
+will return 0.
+They will return -1 on failure, and set
+.Va errno
+to indicate the error.
+.Sh ERRORS
+.Fn pid_begin
+will leave behind a PID file unless:
+.Bl -tag -width Er
+.It Bq Er ECANCELED
+The operation was cancelled due to internal error.
+.It Bq Er EWOULDBLOCK
+The file is already locked by a still-live process and
+.Fa flags
+includes
+.Dv PID_NOBLOCK .
+.El
+.Pp
+The
+.Fn pid_begin
+function may also fail and set errno for any of the errors specified for
+the routines
+.Xr open 2
+or
+.Xr read 2 .
+.Pp
+The
+.Fn pid_end
+function may fail and set errno for any of the errors specified for the
+routine
+.Xr unlink 2 .

--aM3YZ0Iwxop3KEKx--

To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-hackers" in the body of the message




Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20020623165602.GF81018>