[prev in list] [next in list] [prev in thread] [next in thread] 

List:       hurd-bug
Subject:    Re: RFC: [PATCH] Enable olddelta to be NULL in adjtime(3)
From:       Svante Signell <svante.signell () gmail ! com>
Date:       2016-08-30 15:40:00
Message-ID: 1472571600.23956.63.camel () gmail ! com
[Download RAW message or body]

On Tue, 2016-08-30 at 14:45 +0200, Pino Toscano wrote:
> On Tuesday, 30 August 2016 14:32:32 CEST Svante Signell wrote:
> > 
> > The glibc patch sysdeps_mach_hurd_adjtime.c.diff use a dummy struct timeval
> > to
> > avoid calling ___host_adjust_time() in _adjtime() with a NULL third
> > argument.
> > Smarter solutions can probably be found, comments are welcome.
> You don't need to duplicate the whole __host_adjust_time call:
> 
>   struct timeval dummy;
>   ..
>   if (olddelta == NULL)
>     olddelta = &dummy;
> 
> should be enough.

Thanks!

Attached is an updated patch returning also EINVAL if abs(delta) is too large.

Whether the  adjtime()/RPC_host_adjust_time.c:___host_adjust_time() survives a
delta being NULL remains to test. According to the manpage the bug was:
A longstanding bug meant that if delta was specified as NULL, no valid
information about the outstanding clock adjustment was returned in old-delta.
(In this circumstance, adjtime() should return the outstanding clock adjustment,
without changing it.) This bug is fixed on systems with glibc 2.8 or later and
Linux kernel 2.6.26 or later.

["sysdeps_mach_hurd_adjtime.c.diff" (sysdeps_mach_hurd_adjtime.c.diff)]

Index: glibc-2.23/sysdeps/mach/hurd/adjtime.c
===================================================================
--- glibc-2.23.orig/sysdeps/mach/hurd/adjtime.c
+++ glibc-2.23/sysdeps/mach/hurd/adjtime.c
@@ -16,9 +16,13 @@
    <http://www.gnu.org/licenses/>.  */
 
 #include <errno.h>
+#include <limits.h>
 #include <sys/time.h>
 #include <hurd.h>
 
+#define MAX_SEC (INT_MAX / 1000000L - 2)
+#define MIN_SEC (INT_MIN / 1000000L + 2)
+
 /* Adjust the current time of day by the amount in DELTA.
    If OLDDELTA is not NULL, it is filled in with the amount
    of time adjustment remaining to be done from the last `__adjtime' call.
@@ -28,14 +32,22 @@ __adjtime (const struct timeval *delta,
 {
   error_t err;
   mach_port_t hostpriv;
+  struct timeval dummy = {0, 0};
 
   err = __get_privileged_ports (&hostpriv, NULL);
   if (err)
     return __hurd_fail (EPERM);
 
+  if (delta->tv_sec > MAX_SEC || delta->tv_sec < MIN_SEC)
+    return __hurd_fail (EINVAL);
+
+  /* Handle the case when olddelta is NULL, see man adjtime(3) */
+  if (olddelta == NULL)
+    olddelta = &dummy;
+
   err = __host_adjust_time (hostpriv,
 			    /* `time_value_t' and `struct timeval' are in
-                               fact identical with the names changed.  */
+			       fact identical with the names changed.  */
 			    *(time_value_t *) delta,
 			    (time_value_t *) olddelta);
   __mach_port_deallocate (__mach_task_self (), hostpriv);


[prev in list] [next in list] [prev in thread] [next in thread]