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
This commit is contained in:
Mark Johnston 2019-03-27 19:40:18 +00:00
parent 963ae7a63e
commit a273e09cb2
2 changed files with 48 additions and 9 deletions

View File

@ -100,8 +100,9 @@ pidfile_read(int dirfd, const char *filename, pid_t *pidptr)
} }
struct pidfh * 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 pidfh *pfh;
struct stat sb; struct stat sb;
int error, fd, dirfd, dirlen, filenamelen, count; int error, fd, dirfd, dirlen, filenamelen, count;
@ -112,19 +113,22 @@ pidfile_open(const char *path, mode_t mode, pid_t *pidptr)
if (pfh == NULL) if (pfh == NULL)
return (NULL); return (NULL);
if (path == NULL) { if (pathp == NULL) {
dirlen = snprintf(pfh->pf_dir, sizeof(pfh->pf_dir), dirlen = snprintf(pfh->pf_dir, sizeof(pfh->pf_dir),
"/var/run/"); "/var/run/");
filenamelen = snprintf(pfh->pf_filename, filenamelen = snprintf(pfh->pf_filename,
sizeof(pfh->pf_filename), "%s.pid", getprogname()); sizeof(pfh->pf_filename), "%s.pid", getprogname());
} else { } else {
dirlen = snprintf(pfh->pf_dir, sizeof(pfh->pf_dir), if (strlcpy(path, pathp, sizeof(path)) >= sizeof(path)) {
"%s", path); free(pfh);
filenamelen = snprintf(pfh->pf_filename, errno = ENAMETOOLONG;
sizeof(pfh->pf_filename), "%s", path); return (NULL);
}
dirname(pfh->pf_dir); dirlen = strlcpy(pfh->pf_dir, dirname(path),
basename(pfh->pf_filename); 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) || if (dirlen >= (int)sizeof(pfh->pf_dir) ||

View File

@ -263,6 +263,40 @@ test_pidfile_inherited(void)
return (result); 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 { static struct test {
const char *name; const char *name;
const char *(*func)(void); const char *(*func)(void);
@ -271,6 +305,7 @@ static struct test {
{ "pidfile_self", test_pidfile_self }, { "pidfile_self", test_pidfile_self },
{ "pidfile_contested", test_pidfile_contested }, { "pidfile_contested", test_pidfile_contested },
{ "pidfile_inherited", test_pidfile_inherited }, { "pidfile_inherited", test_pidfile_inherited },
{ "pidfile_relative", test_pidfile_relative },
}; };
int int