/dev/clockctl, O_CLOEXEC and forking

To: tech-userlevel%NetBSD.org@localhost
Subject: /dev/clockctl, O_CLOEXEC and forking
From: Alexander Nasonov <alnsn%yandex.ru@localhost>
Date: Sun, 29 Apr 2018 17:53:31 +0100

While looking whether it's possible to change ntpd to work when
chrooted to a file system mounted with the nodev flag, I noticed
that /dev/clockctl is open with O_CLOEXEC and its file descriptor
is kept in a static variable.  I'm not sure it will work after a
fork correctly. ntpd doesn't open /dev/clockctl before forking but
things may change in a future. Any opinions?

PS I attach my proof-of-concept patch that attempts to configure
/dev/clockctl before chrooting ntpd but it doesn't work because
clockctl's file descriptor is closed by the fork.

-- 
Alex

Index: src/external/bsd/ntp/dist/ntpd/ntpd.c
===================================================================
RCS file: /cvsroot/src/external/bsd/ntp/dist/ntpd/ntpd.c,v
retrieving revision 1.15
diff -p -u -u -r1.15 ntpd.c
--- src/external/bsd/ntp/dist/ntpd/ntpd.c	7 Apr 2018 00:19:53 -0000	1.15
+++ src/external/bsd/ntp/dist/ntpd/ntpd.c	29 Apr 2018 16:06:36 -0000
@@ -214,6 +214,10 @@ extern int syscall	(int, ...);
 static volatile int signalled	= 0;
 static volatile int signo	= 0;
 
+#if 1 /* XXX HAVE_CLOCKCTL_CONFIGURE */
+extern int clockctl_configure(void);
+#endif
+
 /* In an ideal world, 'finish_safe()' would declared as noreturn... */
 static	void		finish_safe	(int);
 static	RETSIGTYPE	finish		(int);
@@ -1027,6 +1031,11 @@ getgroup:
 				msyslog(LOG_ERR, "Cannot chdir() to `%s': %m", chrootdir);
 				exit (-1);
 			}
+#if 1			/* XXX HAVE_CLOCKCTL_CONFIGURE */
+			/* open /dev/clockctl while it's still accessible */
+			if (user != NULL)
+				clockctl_configure();
+#endif
 			if (chroot(chrootdir)) {
 				msyslog(LOG_ERR, "Cannot chroot() to `%s': %m", chrootdir);
 				exit (-1);
Index: src/lib/libc/sys/clock_settime.c
===================================================================
RCS file: /cvsroot/src/lib/libc/sys/clock_settime.c,v
retrieving revision 1.12
diff -p -u -u -r1.12 clock_settime.c
--- src/lib/libc/sys/clock_settime.c	15 Oct 2011 23:00:02 -0000	1.12
+++ src/lib/libc/sys/clock_settime.c	29 Apr 2018 16:06:36 -0000
@@ -51,23 +51,23 @@ __RCSID("$NetBSD: clock_settime.c,v 1.12
 
 #include <sys/clockctl.h>
  
-extern int __clockctl_fd;
-
+int clockctl_configure(void);
+int clockctl_is_configured(void);
 int ____clock_settime50(clockid_t, const struct timespec *);
 
 int
 clock_settime(clockid_t clock_id, const struct timespec *tp)
 {
 	struct clockctl_clock_settime args;
-	int rv;
+	int rv, clockctl = -1;
 
 	/*
 	 * always try the syscall first and attempt to switch to
 	 * clockctl if that fails.
 	 */
-	if (__clockctl_fd == -1) {
+	if (!clockctl_is_configured()) {
 		rv = ____clock_settime50(clock_id, tp);
-	
+
 		/*
 		 * return unless we failed with EPERM
 		 */
@@ -78,19 +78,18 @@ clock_settime(clockid_t clock_id, const 
 		 * If this fails, it means that we are not root
 		 * and we cannot open clockctl. This is a failure.
 		 */
-		__clockctl_fd = open(_PATH_CLOCKCTL, O_WRONLY | O_CLOEXEC, 0);
-		if (__clockctl_fd == -1) {
+		if ((clockctl = clockctl_configure()) == -1) {
 			/* original error was EPERM - don't leak open errors */
 			errno = EPERM;
 			return -1;
 		}
 	}
 
-	/* 
-	 * If __clockctl_fd >=0, clockctl has already been open
+	/*
+	 * If we're here, clockctl has already been open
 	 * and used, so we carry on using it.
 	 */
 	args.clock_id = clock_id;
 	args.tp = tp;
-	return ioctl(__clockctl_fd, CLOCKCTL_CLOCK_SETTIME, &args);
+	return ioctl(clockctl, CLOCKCTL_CLOCK_SETTIME, &args);
 }
Index: src/lib/libc/sys/adjtime.c
===================================================================
RCS file: /cvsroot/src/lib/libc/sys/adjtime.c,v
retrieving revision 1.12
diff -p -u -u -r1.12 adjtime.c
--- src/lib/libc/sys/adjtime.c	15 Oct 2011 23:00:02 -0000	1.12
+++ src/lib/libc/sys/adjtime.c	29 Apr 2018 16:06:36 -0000
@@ -48,24 +48,24 @@ __RCSID("$NetBSD: adjtime.c,v 1.12 2011/
 #include <sys/syscall.h>
 
 #include <sys/clockctl.h>
- 
-extern int __clockctl_fd;
 
+int clockctl_configure(void);
+int clockctl_is_configured(void);
 int ____adjtime50(const struct timeval *, struct timeval *);
 
 int
 adjtime(const struct timeval *delta, struct timeval *olddelta)
 {
 	struct clockctl_adjtime args;
-	int rv;
+	int rv, clockctl = -1;
 
 	/*
-	 * we always attempt the syscall first and switch to 
+	 * we always attempt the syscall first and switch to
 	 * clockctl if that fails with EPERM
 	 */
-	if (__clockctl_fd == -1) {
+	if (!clockctl_is_configured()) {
 		rv = ____adjtime50(delta, olddelta);
-	
+
 		/*
 		 * try via clockctl if the call fails with EPERM
 		 */
@@ -76,19 +76,18 @@ adjtime(const struct timeval *delta, str
 		 * If this fails, it means that we are not root
 		 * and we cannot open clockctl. This is a failure.
 		 */
-		__clockctl_fd = open(_PATH_CLOCKCTL, O_WRONLY | O_CLOEXEC, 0);
-		if (__clockctl_fd == -1) {
+		if ((clockctl = clockctl_configure()) == -1) {
 			/* original error was EPERM - don't leak open errors */
 			errno = EPERM;
 			return -1;
 		}
 	}
 
-	/* 
-	 * If __clockctl_fd >=0, clockctl has already been open
+	/*
+	 * If we're, clockctl has already been open
 	 * and used, so we carry on using it.
 	 */
 	args.delta = delta;
 	args.olddelta = olddelta;
-	return ioctl(__clockctl_fd, CLOCKCTL_ADJTIME, &args);
+	return ioctl(clockctl, CLOCKCTL_ADJTIME, &args);
 }
Index: src/lib/libc/sys/ntp_adjtime.c
===================================================================
RCS file: /cvsroot/src/lib/libc/sys/ntp_adjtime.c,v
retrieving revision 1.13
diff -p -u -u -r1.13 ntp_adjtime.c
--- src/lib/libc/sys/ntp_adjtime.c	20 Mar 2012 16:26:12 -0000	1.13
+++ src/lib/libc/sys/ntp_adjtime.c	29 Apr 2018 16:06:36 -0000
@@ -55,8 +55,8 @@ __RCSID("$NetBSD: ntp_adjtime.c,v 1.13 2
 __weak_alias(ntp_adjtime,_ntp_adjtime)
 #endif
 
-extern int __clockctl_fd;
-
+int clockctl_configure(void);
+int clockctl_is_configured(void);
 int __ntp_adjtime(struct timex *);
 
 int
@@ -64,7 +64,7 @@ ntp_adjtime(struct timex *tp)
 {
 	struct clockctl_ntp_adjtime args;
 	int error;
-	int rv;
+	int rv, clockctl = -1;
 
 	/*
 	 * we always attempt to use the syscall unless we had to
@@ -72,9 +72,9 @@ ntp_adjtime(struct timex *tp)
 	 *
 	 * ntp_adjtime() is callable for mortals if tp->modes == 0 !
 	 */
-	if (__clockctl_fd == -1) {
+	if (!clockctl_is_configured()) {
 		rv = __ntp_adjtime(tp);
-	
+
 		/*
 		 * if we fail with EPERM we try the clockctl device
 		 */
@@ -86,8 +86,7 @@ ntp_adjtime(struct timex *tp)
 		 * and we cannot open clockctl. This is a true
 		 * failure.
 		 */
-		__clockctl_fd = open(_PATH_CLOCKCTL, O_WRONLY | O_CLOEXEC, 0);
-		if (__clockctl_fd == -1) {
+		if ((clockctl = clockctl_configure()) == -1) {
 			/* original error was EPERM - don't leak open errors */
 			errno = EPERM;
 			return -1;
@@ -95,11 +94,11 @@ ntp_adjtime(struct timex *tp)
 	}
 
 	/*
-	 * If __clockctl_fd >=0, clockctl has already been open
+	 * If we're, clockctl has already been open
 	 * and used, so we carry on using it.
 	 */
 	args.tp = tp;
-	error = ioctl(__clockctl_fd, CLOCKCTL_NTP_ADJTIME, &args);
+	error = ioctl(clockctl, CLOCKCTL_NTP_ADJTIME, &args);
 
 	/*
 	 * There is no way to get retval set through ioctl(), hence we
Index: src/lib/libc/sys/settimeofday.c
===================================================================
RCS file: /cvsroot/src/lib/libc/sys/settimeofday.c,v
retrieving revision 1.14
diff -p -u -u -r1.14 settimeofday.c
--- src/lib/libc/sys/settimeofday.c	15 Oct 2011 23:00:02 -0000	1.14
+++ src/lib/libc/sys/settimeofday.c	29 Apr 2018 16:06:36 -0000
@@ -50,24 +50,45 @@ __RCSID("$NetBSD: settimeofday.c,v 1.14 
 #include <string.h>
 #include <time.h>
 #include <unistd.h>
- 
-int __clockctl_fd = -1;
 
+static int __clockctl_fd = -1;
+
+int clockctl_configure(void);
+int clockctl_is_configured(void);
 int ____settimeofday50(const struct timeval *, const void *);
 
 int
+clockctl_configure(void)
+{
+
+	if (__clockctl_fd == -1) {
+		__clockctl_fd = open(_PATH_CLOCKCTL, O_WRONLY | O_CLOEXEC, 0);
+		/* XXX Check that __clockctl_fd is usable. */
+	}
+
+	return __clockctl_fd;
+}
+
+int
+clockctl_is_configured(void)
+{
+
+	return __clockctl_fd != -1;
+}
+
+int
 settimeofday(const struct timeval *tv, const void *tzp)
 {
 	struct clockctl_settimeofday args;
-	int rv;
+	int rv, clockctl = -1;
 
 	/*
 	 * try syscal first and attempt to switch to clockctl
 	 * if that fails with EPERM
 	 */
-	if (__clockctl_fd == -1) {
+	if (!clockctl_is_configured()) {
 		rv = ____settimeofday50(tv, tzp);
-	
+
 		/*
 		 * switch to clockctl if we fail with EPERM, this
 		 * may be cause by an attempt to set the time backwards
@@ -77,19 +98,18 @@ settimeofday(const struct timeval *tv, c
 		if (rv != -1 || errno != EPERM)
 			return rv;
 
-		__clockctl_fd = open(_PATH_CLOCKCTL, O_WRONLY | O_CLOEXEC, 0);
-		if (__clockctl_fd == -1) {
+		if ((clockctl = clockctl_configure()) == -1) {
 			/* original error was EPERM - don't leak open errors */
 			errno = EPERM;
 			return -1;
 		}
 	}
 
-	/* 
-	 * If __clockctl_fd >=0, clockctl has already been open
+	/*
+	 * If we're, clockctl has already been open
 	 * and used, so we carry on using it.
 	 */
 	args.tv = tv;
 	args.tzp = tzp;
-	return ioctl(__clockctl_fd, CLOCKCTL_SETTIMEOFDAY, &args);
+	return ioctl(clockctl, CLOCKCTL_SETTIMEOFDAY, &args);
 }

Follow-Ups:
- Re: /dev/clockctl, O_CLOEXEC and forking
  - From: Christos Zoulas

Prev by Date: Re: NetBSD make: determining a relative OBJDIR?
Next by Date: Re: /dev/clockctl, O_CLOEXEC and forking
Previous by Thread: NetBSD make: determining a relative OBJDIR?
Next by Thread: Re: /dev/clockctl, O_CLOEXEC and forking
Indexes:

Home | Main Index | Thread Index | Old Index