Subject: bug#65255: closed (Re: bug#65255: uptime's boot time is inconsistent after VM sleep & resume)

From: [email-masked] (GNU bug Tracking System)
Date: Tue, 15 Aug 2023 21:15:01 +0000

Your bug report

#65255: uptime's boot time is inconsistent after VM sleep & resume

which was filed against the coreutils package, has been closed.

The explanation is attached below, along with your original report.
If you require more details, please reply to [email-masked].

-- 
65255: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=65255
GNU Bug Tracking System
Contact [email-masked] with problems

Subject: Re: bug#65255: uptime's boot time is inconsistent after VM sleep & resume
From: Paul Eggert <[email-masked]>
To: Bruno Haible <[email-masked]>
Cc: [email-masked]
Date: Tue, 15 Aug 2023 14:14:20 -0700
List-Id: <debbugs-submit.debbugs.gnu.org>

Thanks for the further comments. I installed your patch, along with the 
attached additional patches. The first makes 'uptime' a bit more 
resilient in the case of utmp and other failures, and the second adds 
NEWS items as per your comments.

From 1ea34cbf6a235f2436a3265ab9ded6f04748051e Mon Sep 17 00:00:00 2001
From: Paul Eggert <[email-masked]>
Date: Tue, 15 Aug 2023 14:00:54 -0700
Subject: [PATCH 1/2] uptime: be more generous about read_utmp failure

* src/uptime.c (print_uptime): Check for overflow
when computing uptime.  Use C99-style decl after statements.
Do not let an idx_t value go negative.
(print_uptime, uptime): Be more generous about read_utmp failures,
or when read_utmp does not report the boot time.  Instead of
failing, warn but keep going, printing the information that we did
get, and then exit with nonzero status.
(print_uptime): Return the desired exit status.  Caller changed.
---
 src/uptime.c | 77 +++++++++++++++++++++++++++++++---------------------
 1 file changed, 46 insertions(+), 31 deletions(-)

diff --git a/src/uptime.c b/src/uptime.c
index 813d29430..928829dfc 100644
--- a/src/uptime.c
+++ b/src/uptime.c
@@ -17,9 +17,11 @@
 /* Created by hacking who.c by Kaveh Ghazi [email-masked].  */
 
 #include <config.h>
-#include <stdio.h>
 
+#include <stdckdint.h>
+#include <stdio.h>
 #include <sys/types.h>
+
 #include "system.h"
 
 #if HAVE_SYSCTL && HAVE_SYS_SYSCTL_H && ! defined __GLIBC__
@@ -43,19 +45,11 @@
   proper_name ("David MacKenzie"), \
   proper_name ("Kaveh Ghazi")
 
-static void
-print_uptime (idx_t n, struct gl_utmp const *this)
+static int
+print_uptime (idx_t n, struct gl_utmp const *utmp_buf)
 {
-  idx_t entries = 0;
+  int status = EXIT_SUCCESS;
   time_t boot_time = 0;
-  time_t time_now;
-  time_t uptime;
-  intmax_t updays;
-  int uphours;
-  int upmins;
-  struct tm *tmn;
-  double avg[3];
-  int loads;
 
 #if HAVE_SYSCTL && ! defined __GLIBC__ \
     && defined CTL_KERN && defined KERN_BOOTTIME
@@ -81,35 +75,47 @@ print_uptime (idx_t n, struct gl_utmp const *this)
 
   /* Loop through all the utmp entries we just read and count up the valid
      ones, also in the process possibly gleaning boottime. */
-  while (n--)
+  idx_t entries = 0;
+  for (idx_t i = 0; i < n; i++)
     {
+      struct gl_utmp const *this = &utmp_buf[i];
       entries += IS_USER_PROCESS (this);
       if (UT_TYPE_BOOT_TIME (this))
         boot_time = this->ut_ts.tv_sec;
-      ++this;
     }
   /* The gnulib module 'readutmp' is supposed to provide a BOOT_TIME entry
      on all platforms.  */
   if (boot_time == 0)
-    error (EXIT_FAILURE, errno, _("couldn't get boot time"));
-
-  time_now = time (nullptr);
-  uptime = time_now - boot_time;
-  updays = uptime / 86400;
-  uphours = uptime % 86400 / 3600;
-  upmins = uptime % 86400 % 3600 / 60;
-  tmn = localtime (&time_now);
+    {
+      error (0, errno, _("couldn't get boot time"));
+      status = EXIT_FAILURE;
+    }
+
+  time_t time_now = time (nullptr);
+  struct tm *tmn = time_now == (time_t) -1 ? nullptr : localtime (&time_now);
   /* procps' version of uptime also prints the seconds field, but
      previous versions of coreutils don't. */
   if (tmn)
     /* TRANSLATORS: This prints the current clock time. */
     fprintftime (stdout, _(" %H:%M:%S  "), tmn, 0, 0);
   else
-    printf (_(" ??:????  "));
-  if (uptime == (time_t) -1)
-    printf (_("up ???? days ??:??,  "));
+    {
+      printf (_(" ??:????  "));
+      status = EXIT_FAILURE;
+    }
+
+  intmax_t uptime;
+  if (time_now == (time_t) -1 || boot_time == 0
+      || ckd_sub (&uptime, time_now, boot_time) || uptime < 0)
+    {
+      printf (_("up ???? days ??:??,  "));
+      status = EXIT_FAILURE;
+    }
   else
     {
+      intmax_t updays = uptime / 86400;
+      int uphours = uptime % 86400 / 3600;
+      int upmins = uptime % 86400 % 3600 / 60;
       if (0 < updays)
         printf (ngettext ("up %"PRIdMAX" day %2d:%02d,  ",
                           "up %"PRIdMAX" days %2d:%02d,  ",
@@ -118,10 +124,12 @@ print_uptime (idx_t n, struct gl_utmp const *this)
       else
         printf (_("up  %2d:%02d,  "), uphours, upmins);
     }
+
   printf (ngettext ("%td user", "%td users", select_plural (entries)),
           entries);
 
-  loads = getloadavg (avg, 3);
+  double avg[3];
+  int loads = getloadavg (avg, 3);
 
   if (loads == -1)
     putchar ('\n');
@@ -136,6 +144,8 @@ print_uptime (idx_t n, struct gl_utmp const *this)
       if (loads > 0)
         putchar ('\n');
     }
+
+  return status;
 }
 
 /* Display the system uptime and the number of users on the system,
@@ -147,12 +157,17 @@ uptime (char const *filename, int options)
 {
   idx_t n_users;
   struct gl_utmp *utmp_buf;
-  if (read_utmp (filename, &n_users, &utmp_buf, options) != 0)
-    error (EXIT_FAILURE, errno, "%s", quotef (filename));
-
-  print_uptime (n_users, utmp_buf);
+  int read_utmp_status = (read_utmp (filename, &n_users, &utmp_buf, options) < 0
+                          ? EXIT_FAILURE : EXIT_SUCCESS);
+  if (read_utmp_status != EXIT_SUCCESS)
+    {
+      error (0, errno, "%s", quotef (filename));
+      n_users = 0;
+      utmp_buf = nullptr;
+    }
 
-  exit (EXIT_SUCCESS);
+  int print_uptime_status = print_uptime (n_users, utmp_buf);
+  exit (MAX (read_utmp_status, print_uptime_status));
 }
 
 void
-- 
2.39.2

From b1b67827b2e288f1bd8e220adce4333faf6f0cd2 Mon Sep 17 00:00:00 2001
From: Paul Eggert <[email-masked]>
Date: Tue, 15 Aug 2023 14:11:38 -0700
Subject: [PATCH 2/2] maint: update uptime NEWS

* NEWS: Update as per Bruno Haible <https://bugs.gnu.org/65255#14>.
---
 NEWS | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/NEWS b/NEWS
index 3a06f4b30..909dcacd4 100644
--- a/NEWS
+++ b/NEWS
@@ -51,7 +51,10 @@ GNU coreutils NEWS                                    -*- outline -*-
   erroneously, especially with large input files with no separators.
   [This bug was present in "the beginning".]
 
-  'uptime' no longer incorrectly prints "0 users" on OpenBSD.
+  'uptime' no longer incorrectly prints "0 users" on OpenBSD,
+  and is being built again on FreeBSD and Haiku.
+  [bugs introduced in coreutils-9.2]
+
   [bug introduced in coreutils-9.2]
 
   'wc -l' and 'cksum' no longer crash with an "Illegal instruction" error
@@ -91,6 +94,13 @@ GNU coreutils NEWS                                    -*- outline -*-
 
   tac now falls back to '/tmp' if a configured $TMPDIR is unavailable.
 
+  'who -a' now displays the boot time on Alpine Linux, OpenBSD,
+  Cygwin, Haiku, and some Android distributions
+
+  'uptime' now succeeds on some Android distributions, and now counts
+  VM saved/sleep time on GNU (Linux, Hurd, kFreeBSD), NetBSD, OpenBSD,
+  Minux, and Cygwin.
+
   On GNU/Linux platforms where utmp-format files have 32-bit timestamps,
   pinky, uptime, and who can now work for times after the year 2038,
   so long as systemd is installed, you configure with a new, experimental
-- 
2.39.2

Subject: uptime's boot time is inconsistent after VM sleep & resume
From: Bruno Haible <[email-masked]>
To: [email-masked]
Date: Sun, 13 Aug 2023 01:00:37 +0200
List-Id: <debbugs-submit.debbugs.gnu.org>

On the following platforms:
  - Linux (that includes glibc-based systems, Alpine Linux, Raspbian),
  - NetBSD,
  - Cygwin,
  - Minix,

the "up" duration is inconsistent after the VM in which the OS is running
has been
  1. put into sleep / saved / pause mode (terminology depends on the
     hypervisor),
  2. resumed,
  3. a date bump has been done in the VM (either automatically or manually).

VM sleep and a date adjustment after resume is a common operation, as can
be seen
  - from the fact that it's automatic in VirtualBox machines with the
    "guest extensions" installed [1],
  - from the fact that it's automatic in KVM [2],
  - from the fact that it's automatic in Haiku as guest,
  - from a remark in the libvirt documentation [3],
  - from stackoverflow questions such as [4].

After such a VM sleep and a date adjustment, coreutils' "uptime" is
inconsistent in three ways:

  * On Linux, the displayed "up" duration includes active time and
    hardware suspend time [5], but excludes VM sleep time.

    Such a figure is not useful for estimating the boot time, since
    (time now) - (that "up" time) is not the boot time.

    It is also not useful for estimating when e.g. a file system check
    would be necessary on file systems without a journal, or how much
    electricity was consumed — since the hardware suspend time was included.

    So this figure is useless.

  * The displayed "up" duration is inconsistent with the boot time displayed
    by "who -a". (Since the Gnulib module 'readutmp' makes efforts to find
    the boot time in a way that does not change when the date is adjusted.)

  * The behaviour is inconsistent among platforms: On platforms which have
    a /proc/uptime file, the displayed "up" time _excludes_ VM sleep time.
    On the other platforms it _includes_ VM sleep time.

Seen on Linux (Fedora Rawhide, Alpine Linux 3.18, Raspbian), NetBSD 9.3,
Cygwin 2.9.0, Minix 3.3.

I see this as a bug. Find attached a fix for this bug. I'll provide a NEWS
entry afterwards, that summarizes the changes in coreutils + gnulib on the
various platforms.

Bruno

[1] https://docs.oracle.com/en/virtualization/virtualbox/6.1/user/guestadditions.html#4.1.-Introduction-to-Guest-Additions
[2] https://bugzilla.redhat.com/show_bug.cgi?id=1115340
[3] https://libvirt.gitlab.io/libvirt-appdev-guide-python/libvirt_application_development_guide_using_python-Guest_Domains-Lifecycle-Save.html
[4] https://serverfault.com/questions/334698/
[5] This is indicated by glibc's <bits/time.h>:
      /* Monotonic system-wide clock that includes time spent in suspension.  */
      # define CLOCK_BOOTTIME                 7
    I also verified this on a laptop.
>From ff6e92f38b560ade54df4c94e9d91657f740bb4b Mon Sep 17 00:00:00 2001
From: Bruno Haible <[email-masked]>
Date: Sun, 13 Aug 2023 00:57:23 +0200
Subject: [PATCH] uptime: Include VM sleep time in the "up" duration

* src/uptime.c: Don't include c-strtod.h.
(print_uptime): Don't read /proc/uptime, because the value it provides
does not change when a date adjustment occurs.
* bootstrap.conf (gnulib_modules): Remove 'uptime'.
---
 bootstrap.conf |  1 -
 src/uptime.c   | 36 ++++++------------------------------
 2 files changed, 6 insertions(+), 31 deletions(-)

diff --git a/bootstrap.conf b/bootstrap.conf
index 6ce27d5dd..d758a9ff6 100644
--- a/bootstrap.conf
+++ b/bootstrap.conf
@@ -285,7 +285,6 @@ gnulib_modules="
   unlocked-io
   unsetenv
   update-copyright
-  uptime
   useless-if-before-free
   userspec
   utimecmp
diff --git a/src/uptime.c b/src/uptime.c
index c29798d32..0cc1dedba 100644
--- a/src/uptime.c
+++ b/src/uptime.c
@@ -22,7 +22,6 @@
 #include <sys/types.h>
 #include "system.h"
 
-#include "c-strtod.h"
 #include "long-options.h"
 #include "quote.h"
 #include "readutmp.h"
@@ -42,33 +41,13 @@ print_uptime (idx_t n, struct gl_utmp const *this)
   idx_t entries = 0;
   time_t boot_time = 0;
   time_t time_now;
-  time_t uptime = 0;
+  time_t uptime;
   intmax_t updays;
   int uphours;
   int upmins;
   struct tm *tmn;
   double avg[3];
   int loads;
-#ifdef HAVE_PROC_UPTIME
-  FILE *fp;
-
-  fp = fopen ("/proc/uptime", "r");
-  if (fp != nullptr)
-    {
-      char buf[BUFSIZ];
-      char *b = fgets (buf, BUFSIZ, fp);
-      if (b == buf)
-        {
-          char *end_ptr;
-          double upsecs = c_strtod (buf, &end_ptr);
-          if (buf != end_ptr)
-            uptime = (0 <= upsecs && upsecs < TYPE_MAXIMUM (time_t)
-                      ? upsecs : -1);
-        }
-
-      fclose (fp);
-    }
-#endif /* HAVE_PROC_UPTIME */
 
   /* Loop through all the utmp entries we just read and count up the valid
      ones, also in the process possibly gleaning boottime. */
@@ -79,16 +58,13 @@ print_uptime (idx_t n, struct gl_utmp const *this)
         boot_time = this->ut_ts.tv_sec;
       ++this;
     }
+  /* The gnulib module 'readutmp' is supposed to provide a BOOT_TIME entry
+     on all platforms.  */
+  if (boot_time == 0)
+    error (EXIT_FAILURE, errno, _("couldn't get boot time"));
 
   time_now = time (nullptr);
-#if defined HAVE_PROC_UPTIME
-  if (uptime == 0)
-#endif
-    {
-      if (boot_time == 0)
-        error (EXIT_FAILURE, errno, _("couldn't get boot time"));
-      uptime = time_now - boot_time;
-    }
+  uptime = time_now - boot_time;
   updays = uptime / 86400;
   uphours = uptime % 86400 / 3600;
   upmins = uptime % 86400 % 3600 / 60;
-- 
2.34.1