Subject: clockctl support in libc
To: None <tech-kern@netbsd.org>
From: Frank Kardel <kardel@netbsd.org>
List: tech-kern
Date: 09/23/2006 20:06:16
This is a multi-part message in MIME format.
--------------070807020904070801040502
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit
Hi,
during tests I found that our clockctl support in libc for the time
controlling functions is sometimes a bit strange or even wrong.
The strange parts come up now as we are moving towards security
models. Given that, we shouldn't test for geteuid() == 0 any more but
rather try the syscall unconditionally and only try to switch to clockctl
when we get an EPERM and go from there. This simplyfies the code a bit.
One problem is that if root attempts to set time backwards at higher
securelevels we cannot distinguish between EPERM because of limited
rights or the policy violation - we would switch to clockctl and fail
there again.
Additionally I didn't see any errno adjustments to EPERM when
open(__PATH_CLOCKCTL, ...) fails - thus we return an open error code
instead of EPERM right now - this does not agree with common expectations
and manuals.
The plain wrong part is in libc's ntp_adjtime() - a call with struct
timex mode == 0
is allowed for any mortal user to get information. This currently fails
for mortals when
these have no rights for __PATH_CLOCKCTL. this is plain wrong.
I'd like to fix that patch (except for errno adjustment) attached. Any
comments ?
Frank
--------------070807020904070801040502
Content-Type: text/plain;
name="p"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
filename="p"
Index: adjtime.c
===================================================================
RCS file: /cvsroot/src/lib/libc/sys/adjtime.c,v
retrieving revision 1.7
diff -u -r1.7 adjtime.c
--- adjtime.c 9 Mar 2006 23:44:43 -0000 1.7
+++ adjtime.c 23 Sep 2006 18:02:44 -0000
@@ -66,11 +66,10 @@
int rv;
/*
- * if __clockctl_fd == -1, then this is not our first time,
- * and we know root is the calling user. We use the system call
+ * we always attempt the syscall first and switch to
+ * clockctl if that fails with EPERM
*/
if (__clockctl_fd == -1) {
-try_syscall:
q = __syscall((quad_t)SYS_adjtime, delta, olddelta);
if (/* LINTED constant */ sizeof (quad_t) == sizeof (register_t)
|| /* LINTED constant */ BYTE_ORDER == LITTLE_ENDIAN)
@@ -79,30 +78,10 @@
rv = (int)((u_quad_t)q >> 32);
/*
- * If credentials changed from root to an unprivilegied
- * user, and we already had __clockctl_fd = -1, then we
- * tried the system call as a non root user, it failed
- * with EPERM, and we will try clockctl.
+ * try via clockctl if the call fails with EPERM
*/
if (rv != -1 || errno != EPERM)
return rv;
- __clockctl_fd = -2;
- }
-
- /*
- * If __clockctl_fd = -2 then this is our first time here,
- * or credentials have changed (the calling process dropped root
- * root privilege). Check if root is the calling user. If it is,
- * we try the system call, if it is not, we try clockctl.
- */
- if (__clockctl_fd == -2) {
- /*
- * Root always uses the syscall
- */
- if (geteuid() == 0) {
- __clockctl_fd = -1;
- goto try_syscall;
- }
/*
* If this fails, it means that we are not root
@@ -111,6 +90,7 @@
__clockctl_fd = open(_PATH_CLOCKCTL, O_WRONLY, 0);
if (__clockctl_fd == -1)
return -1;
+
(void) fcntl(__clockctl_fd, F_SETFD, FD_CLOEXEC);
}
Index: clock_settime.c
===================================================================
RCS file: /cvsroot/src/lib/libc/sys/clock_settime.c,v
retrieving revision 1.7
diff -u -r1.7 clock_settime.c
--- clock_settime.c 9 Mar 2006 23:44:43 -0000 1.7
+++ clock_settime.c 23 Sep 2006 18:02:44 -0000
@@ -68,11 +68,10 @@
int rv;
/*
- * if __clockctl_fd == -1, then this is not our first time,
- * and we know root is the calling user. We use the system call
+ * always try the syscall first and attempt to switch to
+ * clockctl if that fails.
*/
if (__clockctl_fd == -1) {
-try_syscall:
q = __syscall((quad_t)SYS_clock_settime, clock_id, tp);
if (/* LINTED constant */ sizeof (quad_t) == sizeof (register_t)
|| /* LINTED constant */ BYTE_ORDER == LITTLE_ENDIAN)
@@ -81,30 +80,10 @@
rv = (int)((u_quad_t)q >> 32);
/*
- * If credentials changed from root to an unprivilegied
- * user, and we already had __clockctl_fd = -1, then we
- * tried the system call as a non root user, it failed
- * with EPERM, and we will try clockctl.
+ * return unless we failed with EPERM
*/
if (rv != -1 || errno != EPERM)
return rv;
- __clockctl_fd = -2;
- }
-
- /*
- * If __clockctl_fd = -2 then this is our first time here,
- * or credentials have changed (the calling process dropped root
- * root privilege). Check if root is the calling user. If it is,
- * we try the system call, if it is not, we try clockctl.
- */
- if (__clockctl_fd == -2) {
- /*
- * Root always uses the syscall
- */
- if (geteuid() == 0) {
- __clockctl_fd = -1;
- goto try_syscall;
- }
/*
* If this fails, it means that we are not root
@@ -113,6 +92,7 @@
__clockctl_fd = open(_PATH_CLOCKCTL, O_WRONLY, 0);
if (__clockctl_fd == -1)
return -1;
+
(void) fcntl(__clockctl_fd, F_SETFD, FD_CLOEXEC);
}
Index: ntp_adjtime.c
===================================================================
RCS file: /cvsroot/src/lib/libc/sys/ntp_adjtime.c,v
retrieving revision 1.7
diff -u -r1.7 ntp_adjtime.c
--- ntp_adjtime.c 9 Mar 2006 23:44:43 -0000 1.7
+++ ntp_adjtime.c 23 Sep 2006 18:02:44 -0000
@@ -70,11 +70,12 @@
int rv;
/*
- * if __clockctl_fd == -1, then this is not our first time,
- * and we know root is the calling user. We use the system call
+ * we always attempt to use the syscall unless we had to
+ * use the clockctl device before
+ *
+ * ntp_adjtime() is callable for mortals if tp->modes == 0 !
*/
if (__clockctl_fd == -1) {
-try_syscall:
q = __syscall((quad_t)SYS_ntp_adjtime, tp);
if (/* LINTED constant */ sizeof (quad_t) == sizeof (register_t)
|| /* LINTED constant */ BYTE_ORDER == LITTLE_ENDIAN)
@@ -83,38 +84,20 @@
rv = (int)((u_quad_t)q >> 32);
/*
- * If credentials changed from root to an unprivilegied
- * user, and we already had __clockctl_fd = -1, then we
- * tried the system call as a non root user, it failed
- * with EPERM, and we will try clockctl.
+ * if we fail with EPERM we try the clockctl device
*/
if (rv != -1 || errno != EPERM)
return rv;
- __clockctl_fd = -2;
- }
-
- /*
- * If __clockctl_fd = -2 then this is our first time here,
- * or credentials have changed (the calling process dropped root
- * root privilege). Check if root is the calling user. If it is,
- * we try the system call, if it is not, we try clockctl.
- */
- if (__clockctl_fd == -2) {
- /*
- * Root always uses the syscall
- */
- if (geteuid() == 0) {
- __clockctl_fd = -1;
- goto try_syscall;
- }
/*
* If this fails, it means that we are not root
- * and we cannot open clockctl. This is a failure.
+ * and we cannot open clockctl. This is a true
+ * failure.
*/
__clockctl_fd = open(_PATH_CLOCKCTL, O_WRONLY, 0);
if (__clockctl_fd == -1)
return -1;
+
(void) fcntl(__clockctl_fd, F_SETFD, FD_CLOEXEC);
}
@@ -133,6 +116,6 @@
rv = (int)args.retval;
return rv;
}
- return error;
+ return error;
}
Index: settimeofday.c
===================================================================
RCS file: /cvsroot/src/lib/libc/sys/settimeofday.c,v
retrieving revision 1.9
diff -u -r1.9 settimeofday.c
--- settimeofday.c 17 Aug 2006 09:59:55 -0000 1.9
+++ settimeofday.c 23 Sep 2006 18:02:44 -0000
@@ -55,7 +55,7 @@
__weak_alias(settimeofday,_settimeofday)
#endif
-int __clockctl_fd = -2;
+int __clockctl_fd = -1;
int
settimeofday(tv, tzp)
@@ -68,11 +68,10 @@
int rv;
/*
- * if __clockctl_fd == -1, then this is not our first time,
- * and we know root is the calling user. We use the system call
+ * try syscal first and attempt to switch to clockctl
+ * if that fails with EPERM
*/
if (__clockctl_fd == -1) {
-try_syscall:
q = __syscall((quad_t)SYS_settimeofday, tv, tzp);
if (/* LINTED constant */ sizeof (quad_t) == sizeof (register_t)
|| /* LINTED constant */ BYTE_ORDER == LITTLE_ENDIAN)
@@ -81,41 +80,18 @@
rv = (int)((u_quad_t)q >> 32);
/*
- * If credentials changed from root to an unprivilegied
- * user, and we already had __clockctl_fd = -1, then we
- * tried the system call as a non root user, it failed
- * with EPERM, and we will try clockctl unless the user
- * is root which means they probably tried to set the
- * clock backwards and are not allowed to do so because
- * of the securelevel.
+ * switch to clockctl if we fail with EPERM, this
+ * may be cause by an attempt to set the time backwards
+ * but we should leave the access permission checking
+ * entirely to the kernel
*/
- if (rv != -1 || errno != EPERM || geteuid() == 0)
+ if (rv != -1 || errno != EPERM)
return rv;
- __clockctl_fd = -2;
- }
-
- /*
- * If __clockctl_fd = -2 then this is our first time here,
- * or credentials have changed (the calling process dropped root
- * root privilege). Check if root is the calling user. If it is,
- * we try the system call, if it is not, we try clockctl.
- */
- if (__clockctl_fd == -2) {
- /*
- * Root always uses the syscall
- */
- if (geteuid() == 0) {
- __clockctl_fd = -1;
- goto try_syscall;
- }
- /*
- * 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, 0);
if (__clockctl_fd == -1)
return -1;
+
(void) fcntl(__clockctl_fd, F_SETFD, FD_CLOEXEC);
}
--------------070807020904070801040502--