Skip to contentSkip to conversation

hwclock: avoid UB if TZUTC is unset; include sys/time.h for timeval

PATCH New Isaac Dunham 3 participants 5 messages Apr 23, 2015 – Apr 27, 2015 Util-Linux package

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 hwclock: avoid UB if TZUTC is unset; include sys/time.h for timeval

Fix a crash reported by Jean-Marc Pigeon on the musl mailing list.

If TZUTC is unset, setenv() will be called with value = NULL, which invokes undefined behavior.
With musl libc, this causes a crash.
On uclibc and glibc, this currently happens to act like the long form here, but glibc will make this an error:
https://sourceware.org/ml/libc-alpha/2015-03/threads.html#00449

Additionally, include <sys/time.h> to define struct timeval.

---
 sys-utils/hwclock.c | 5 ++++-
 sys-utils/hwclock.h | 1 +
 2 files changed, 5 insertions(+), 1 deletion(-)
sys-utils/hwclock.c +4 -1
index e1e5816..67971f0 100644
@@ -408,7 +408,10 @@ mktime_tz(struct tm tm, const bool universal,
 		 * libc's:
 		 *    TZUTC=:/usr/share/zoneinfo/right/UTC
 		 */
-		setenv("TZ", getenv("TZUTC"), TRUE);
+		char *tzutc;
+
+		tzutc = getenv("TZUTC");
+		setenv("TZ", tzutc ? tzutc : "", TRUE);
 		/*
 		 * Note: tzset() gets called implicitly by the time code,
 		 * but only the first time. When changing the environment
sys-utils/hwclock.h +1 -0
index 974d96a..7f19c39 100644
@@ -6,6 +6,7 @@
 #include <stdlib.h>
 #include <string.h>
 #include <time.h>
+#include <sys/time.h>
 
 #include "c.h"
 
Signature · 1 line
2.3.5
#1 lore raw

cover hwclock: Remove TZUTC

The patch that introduced TZUTC should have been rejected. Mr. Parmelee misunderstood how to use and configure the zoneinfo 'right' database. This was likely due to the 'right' database being undocumented. I added information about the database to the hwclock(8) man-page, and intended to remove the TZUTC environment variable from hwclock when refactoring. Since this regression needs to be addressed, it might as well be removed now.

TZUTC is undocumented and should be safe to remove.

The following changes since commit e2381bdac86bb5fc27fe12ff9994268ea68888dc:

tests: fix sfdisk GPT test (2015-04-23 15:08:26 +0200)

are available in the git repository at:

[email protected]:jwpi/util-linux.git 423

for you to fetch changes up to 6b32ffc5f04869da3f8b4460f216e320df0f911a:

hwclock: Remove TZUTC (2015-04-23 16:26:25 -0400)

----------------------------------------------------------------
J William Piggott (1):
hwclock: Remove TZUTC

 sys-utils/hwclock.c | 11 ++---------
 1 file changed, 2 insertions(+), 9 deletions(-)
#2 lore raw

PATCH 1/1 hwclock: Remove TZUTC

commit d53f8ecfbf036eddb2aef737dc0973a613d80ced introduced the TZUTC environment variable to facilitate use of the zoneinfo 'right' database. This was incorrect. Either the TZDIR environment variable should be used or a system-wide configuration for the 'right' database should be used.

See hwclock(8) POSIX vs 'RIGHT' for more details.

TZUTC was undocumented and should be safe to remove.

The commit also caused a regression when using musl libc, because when TZUTC is unset getenv() returns a NULL pointer.

---
 sys-utils/hwclock.c | 11 ++---------
 1 file changed, 2 insertions(+), 9 deletions(-)
sys-utils/hwclock.c +2 -9
index e1e5816..f4784a1 100644
@@ -400,15 +400,8 @@ mktime_tz(struct tm tm, const bool universal,
 	 */
 	zone = getenv("TZ");	/* remember original time zone */
 	if (universal) {
-		/* Set timezone to UTC as defined by the environment
-		 * variable TZUTC.  TZUTC undefined gives the default UTC
-		 * zonefile which usually does not take into account leap
-		 * seconds.  Define TZUTC to select your UTC zonefile which
-		 * does include leap seconds.  For example, with recent GNU
-		 * libc's:
-		 *    TZUTC=:/usr/share/zoneinfo/right/UTC
-		 */
-		setenv("TZ", getenv("TZUTC"), TRUE);
+		/* Set timezone to UTC */
+		setenv("TZ", "", TRUE);
 		/*
 		 * Note: tzset() gets called implicitly by the time code,
 		 * but only the first time. When changing the environment
maintainer Applied
#3 lore raw

Re: [PATCH 1/1] hwclock: Remove TZUTC

Quoted 2 lines · J William Piggott
 sys-utils/hwclock.c | 11 ++---------
 1 file changed, 2 insertions(+), 9 deletions(-)

Applied, thanks.

Signature · 2 lines
 Karel Zak  <[email protected]>
 http://karelzak.blogspot.com
#4 lore raw

Re: [PATCH 0/1] hwclock: Remove TZUTC

In reply to J William Piggott
Quoted 10 lines · J William Piggott

The patch that introduced TZUTC should have been rejected. Mr. Parmelee misunderstood how to use and configure the zoneinfo 'right' database. This was likely due to the 'right' database being undocumented. I added information about the database to the hwclock(8) man-page, and intended to remove the TZUTC environment variable from hwclock when refactoring. Since this regression needs to be addressed, it might as well be removed now.

Quoted 2 lines

TZUTC is undocumented and should be safe to remove.

Thanks.
This seems like the better fix.

Thank you,
Isaac Dunham