svn commit: r345596 - in head/lib/libutil: . tests

[Mark Johnston]

Author: markj
Date: Wed Mar 27 19:40:18 2019
New Revision: 345596
URL: https://svnweb.freebsd.org/changeset/base/345596

Log:
  Fix pidfile_open(3) to handle relative paths with multiple components.
  
  r322369's use of basename(3) was incorrect and worked by accident so
  long as the pidfile path was absolute or consisted of a single
  component.  Fix the basename() usage and add a regression test.
  
  Reported by:	0mp
  Reviewed by:	cem
  MFC after:	3 days
  Differential Revision:	https://reviews.freebsd.org/D19728

Modified:
  head/lib/libutil/pidfile.c
  head/lib/libutil/tests/pidfile_test.c

Modified: head/lib/libutil/pidfile.c
==============================================================================
--- head/lib/libutil/pidfile.c	Wed Mar 27 19:37:50 2019	(r345595)
+++ head/lib/libutil/pidfile.c	Wed Mar 27 19:40:18 2019	(r345596)
@@ -100,8 +100,9 @@ pidfile_read(int dirfd, const char *filename, pid_t *p
 }
 
 struct pidfh *
-pidfile_open(const char *path, mode_t mode, pid_t *pidptr)
+pidfile_open(const char *pathp, mode_t mode, pid_t *pidptr)
 {
+	char path[MAXPATHLEN];
 	struct pidfh *pfh;
 	struct stat sb;
 	int error, fd, dirfd, dirlen, filenamelen, count;
@@ -112,19 +113,22 @@ pidfile_open(const char *path, mode_t mode, pid_t *pid
 	if (pfh == NULL)
 		return (NULL);
 
-	if (path == NULL) {
+	if (pathp == NULL) {
 		dirlen = snprintf(pfh->pf_dir, sizeof(pfh->pf_dir),
 		    "/var/run/");
 		filenamelen = snprintf(pfh->pf_filename,
 		    sizeof(pfh->pf_filename), "%s.pid", getprogname());
 	} else {
-		dirlen = snprintf(pfh->pf_dir, sizeof(pfh->pf_dir),
-		    "%s", path);
-		filenamelen = snprintf(pfh->pf_filename,
-		    sizeof(pfh->pf_filename), "%s", path);
-
-		dirname(pfh->pf_dir);
-		basename(pfh->pf_filename);
+		if (strlcpy(path, pathp, sizeof(path)) >= sizeof(path)) {
+			free(pfh);
+			errno = ENAMETOOLONG;
+			return (NULL);
+		}
+		dirlen = strlcpy(pfh->pf_dir, dirname(path),
+		    sizeof(pfh->pf_dir));
+		(void)strlcpy(path, pathp, sizeof(path));
+		filenamelen = strlcpy(pfh->pf_filename, basename(path),
+		    sizeof(pfh->pf_filename));
 	}
 
 	if (dirlen >= (int)sizeof(pfh->pf_dir) ||

Modified: head/lib/libutil/tests/pidfile_test.c
==============================================================================
--- head/lib/libutil/tests/pidfile_test.c	Wed Mar 27 19:37:50 2019	(r345595)
+++ head/lib/libutil/tests/pidfile_test.c	Wed Mar 27 19:40:18 2019	(r345596)
@@ -263,6 +263,40 @@ test_pidfile_inherited(void)
 	return (result);
 }
 
+/*
+ * Make sure we handle relative pidfile paths correctly.
+ */
+static const char *
+test_pidfile_relative(void)
+{
+	char path[PATH_MAX], pid[32], tmpdir[PATH_MAX];
+	struct pidfh *pfh;
+	int fd;
+
+	(void)snprintf(tmpdir, sizeof(tmpdir), "%s.XXXXXX", __func__);
+	if (mkdtemp(tmpdir) == NULL)
+		return (strerror(errno));
+	(void)snprintf(path, sizeof(path), "%s/pidfile", tmpdir);
+
+	pfh = pidfile_open(path, 0600, NULL);
+	if (pfh == NULL)
+		return (strerror(errno));
+	if (pidfile_write(pfh) != 0)
+		return (strerror(errno));
+	fd = open(path, O_RDONLY);
+	if (fd < 0)
+		return (strerror(errno));
+	if (read(fd, pid, sizeof(pid)) < 0)
+		return (strerror(errno));
+	if (atoi(pid) != getpid())
+		return ("pid mismatch");
+	if (close(fd) != 0)
+		return (strerror(errno));
+	if (pidfile_close(pfh) != 0)
+		return (strerror(errno));
+	return (NULL);
+}
+
 static struct test {
 	const char *name;
 	const char *(*func)(void);
@@ -271,6 +305,7 @@ static struct test {
 	{ "pidfile_self", test_pidfile_self },
 	{ "pidfile_contested", test_pidfile_contested },
 	{ "pidfile_inherited", test_pidfile_inherited },
+	{ "pidfile_relative", test_pidfile_relative },
 };
 
 int