[PATCH v2 2/4] time: Push tzset lock into callers of time functions
Florian Weimer
fweimer@redhat.com
Thu Jan 2 17:42:39 GMT 2025
This fixes a harmless data race on the cached offset used
by __mktime_internal.
---
include/time.h | 23 --------------------
time/gmtime.c | 8 +++++--
time/localtime.c | 8 +++++--
time/mktime-internal.h | 4 +++-
time/mktime.c | 48 ++++++++++++++++++++++++++++--------------
time/strftime_l.c | 1 +
time/timegm.c | 11 ++++++++++
time/tzfile.c | 1 +
time/tzset.c | 20 +++++++++++-------
time/tzset.h | 34 ++++++++++++++++++++++++++++++
10 files changed, 106 insertions(+), 52 deletions(-)
create mode 100644 time/tzset.h
diff --git a/include/time.h b/include/time.h
index f599eeed4e..e9c083646a 100644
--- a/include/time.h
+++ b/include/time.h
@@ -49,24 +49,6 @@ extern const unsigned short int __mon_yday[2][13] attribute_hidden;
/* Defined in localtime.c. */
extern struct tm _tmbuf attribute_hidden;
-/* Defined in tzset.c. */
-extern char *__tzstring (const char *string) attribute_hidden;
-
-extern int __use_tzfile attribute_hidden;
-
-extern void __tzfile_read (const char *file, size_t extra,
- char **extrap) attribute_hidden;
-extern void __tzfile_compute (__time64_t timer, int use_localtime,
- long int *leap_correct, int *leap_hit,
- struct tm *tp) attribute_hidden;
-extern void __tzfile_default (const char *std, const char *dst,
- int stdoff, int dstoff)
- attribute_hidden;
-extern void __tzset_parse_tz (const char *tz) attribute_hidden;
-extern void __tz_compute (__time64_t timer, struct tm *tm, int use_localtime)
- __THROW attribute_hidden;
-
-
#if __TIMESIZE == 64
# define __itimerspec64 itimerspec
#else
@@ -275,11 +257,6 @@ extern int __offtime (__time64_t __timer,
extern char *__asctime_r (const struct tm *__tp, char *__buf)
attribute_hidden;
-extern void __tzset (void) attribute_hidden;
-
-/* Prototype for the internal function to get information based on TZ. */
-extern struct tm *__tz_convert (__time64_t timer, int use_localtime,
- struct tm *tp) attribute_hidden;
extern int __nanosleep (const struct timespec *__requested_time,
struct timespec *__remaining);
diff --git a/time/gmtime.c b/time/gmtime.c
index ad6e8eb352..9d0af33401 100644
--- a/time/gmtime.c
+++ b/time/gmtime.c
@@ -17,13 +17,17 @@
<https://www.gnu.org/licenses/>. */
#include <time.h>
+#include <tzset.h>
/* Return the `struct tm' representation of *T in UTC,
using *TP to store the result. */
struct tm *
__gmtime64_r (const __time64_t *t, struct tm *tp)
{
- return __tz_convert (*t, 0, tp);
+ __libc_lock_lock (__tzset_lock);
+ struct tm *result = __tz_convert (*t, 0, tp);
+ __libc_lock_unlock (__tzset_lock);
+ return result;
}
/* Provide a 32-bit variant if needed. */
@@ -48,7 +52,7 @@ weak_alias (__gmtime_r, gmtime_r)
struct tm *
__gmtime64 (const __time64_t *t)
{
- return __tz_convert (*t, 0, &_tmbuf);
+ return __gmtime64_r (t, &_tmbuf);
}
/* Provide a 32-bit variant if needed. */
diff --git a/time/localtime.c b/time/localtime.c
index d1967ff11b..7f865a7053 100644
--- a/time/localtime.c
+++ b/time/localtime.c
@@ -17,6 +17,7 @@
<https://www.gnu.org/licenses/>. */
#include <time.h>
+#include <tzset.h>
/* C89 says that localtime and gmtime return the same pointer.
Although C99 and later relax this to let localtime and gmtime
@@ -30,7 +31,10 @@ struct tm _tmbuf;
struct tm *
__localtime64_r (const __time64_t *t, struct tm *tp)
{
- return __tz_convert (*t, 1, tp);
+ __libc_lock_lock (__tzset_lock);
+ struct tm *result = __tz_convert (*t, 1, tp);
+ __libc_lock_unlock (__tzset_lock);
+ return result;
}
/* Provide a 32-bit variant if needed. */
@@ -54,7 +58,7 @@ weak_alias (__localtime_r, localtime_r)
struct tm *
__localtime64 (const __time64_t *t)
{
- return __tz_convert (*t, 1, &_tmbuf);
+ return __localtime64_r (t, &_tmbuf);
}
libc_hidden_def (__localtime64)
diff --git a/time/mktime-internal.h b/time/mktime-internal.h
index 64dc35b3cf..4c3db637ef 100644
--- a/time/mktime-internal.h
+++ b/time/mktime-internal.h
@@ -77,6 +77,8 @@ tz_convert (long_int t, bool use_local, struct tm *tm)
/* Subroutine of mktime. Return the time_t representation of TP and
normalize TP, in local time if USE_LOCAL. Record next guess for
- localtime-gmtime offset in *OFFSET. */
+ localtime-gmtime offset in *OFFSET.
+
+ If _LIBC, the caller must lock __tzset_lock. */
extern __time64_t __mktime_internal (struct tm *tp, bool use_local,
mktime_offset_t *offset) attribute_hidden;
diff --git a/time/mktime.c b/time/mktime.c
index 2a1a925b75..1b294872e1 100644
--- a/time/mktime.c
+++ b/time/mktime.c
@@ -42,6 +42,9 @@
#endif
#include <time.h>
+#ifdef _LIBC
+# include <tzset.h>
+#endif
#include <errno.h>
#include <limits.h>
@@ -64,11 +67,14 @@
#include "mktime-internal.h"
-#if !defined _LIBC && (NEED_MKTIME_WORKING || NEED_MKTIME_WINDOWS)
+#ifndef _LIBC
+# define __libc_lock_lock(lock) (void) 0
+# define __libc_lock_unlock(lock) (void) 0
+# if (NEED_MKTIME_WORKING || NEED_MKTIME_WINDOWS)
static void
my_tzset (void)
{
-# if NEED_MKTIME_WINDOWS
+# if NEED_MKTIME_WINDOWS
/* Rectify the value of the environment variable TZ.
There are four possible kinds of such values:
- Traditional US time zone names, e.g. "PST8PDT". Syntax: see
@@ -94,13 +100,16 @@ my_tzset (void)
const char *tz = getenv ("TZ");
if (tz != NULL && strchr (tz, '/') != NULL)
_putenv ("TZ=");
-# else
+# else /* !NEED_MKTIME_WINDOWS */
tzset ();
-# endif
+# endif
}
-# undef __tzset
-# define __tzset() my_tzset ()
-#endif
+# undef __tzset
+# define __tzset_unlocked() my_tzset ()
+# else /* !(NEED_MKTIME_WORKING || NEED_MKTIME_WINDOWS) */
+# define __tzset_unlocked() tzset ()
+# endif
+#endif /* !_LIBC */
#if defined _LIBC || NEED_MKTIME_WORKING || NEED_MKTIME_INTERNAL
@@ -305,7 +314,9 @@ ranged_convert (long_int *t, bool use_local, struct tm *tp)
If *OFFSET's guess is correct, only one conversion call is needed.
If successful, set *TP to the canonicalized struct tm;
otherwise leave *TP alone, return ((time_t) -1) and set errno.
- This function is external because it is used also by timegm.c. */
+ This function is external because it is used also by timegm.c.
+
+ If _LIBC, the caller must lock __tzset_lock. */
__time64_t
__mktime_internal (struct tm *tp, bool use_local, mktime_offset_t *offset)
{
@@ -519,23 +530,28 @@ __mktime_internal (struct tm *tp, bool use_local, mktime_offset_t *offset)
#endif /* _LIBC || NEED_MKTIME_WORKING || NEED_MKTIME_INTERNAL */
#if defined _LIBC || NEED_MKTIME_WORKING || NEED_MKTIME_WINDOWS
-
/* Convert *TP to a __time64_t value. */
__time64_t
__mktime64 (struct tm *tp)
{
+ __time64_t result;
+
/* POSIX.1 8.1.1 requires that whenever mktime() is called, the
time zone abbreviations contained in the external variable 'tzname' shall
be set as if the tzset() function had been called. */
- __tzset ();
+ __libc_lock_lock (__tzset_lock);
+ __tzset_unlocked ();
-# if defined _LIBC || NEED_MKTIME_WORKING
+# ifdef NEED_MKTIME_WORKING
static mktime_offset_t localtime_offset;
- return __mktime_internal (tp, true, &localtime_offset);
-# else
-# undef mktime
- return mktime (tp);
-# endif
+ result = __mktime_internal (tp, true, &localtime_offset);
+# else
+# undef mktime
+ result = mktime (tp);
+# endif
+
+ __libc_lock_unlock (__tzset_lock);
+ return result;
}
#endif /* _LIBC || NEED_MKTIME_WORKING || NEED_MKTIME_WINDOWS */
diff --git a/time/strftime_l.c b/time/strftime_l.c
index f51d926b46..584df83d15 100644
--- a/time/strftime_l.c
+++ b/time/strftime_l.c
@@ -33,6 +33,7 @@
# define MULTIBYTE_IS_FORMAT_SAFE 1
# define STDC_HEADERS 1
# include "../locale/localeinfo.h"
+# include <tzset.h>
#endif
#if defined emacs && !defined HAVE_BCOPY
diff --git a/time/timegm.c b/time/timegm.c
index ac3f98036d..3506ab5a18 100644
--- a/time/timegm.c
+++ b/time/timegm.c
@@ -24,6 +24,10 @@
#include <time.h>
#include <errno.h>
+#ifdef _LIBC
+# include <tzset.h>
+#endif
+
#include "mktime-internal.h"
__time64_t
@@ -31,7 +35,14 @@ __timegm64 (struct tm *tmp)
{
static mktime_offset_t gmtime_offset;
tmp->tm_isdst = 0;
+#ifdef _LIBC
+ __libc_lock_lock (__tzset_lock);
+ __time64_t result = __mktime_internal (tmp, false, &gmtime_offset);
+ __libc_lock_unlock (__tzset_lock);
+ return result;
+#else
return __mktime_internal (tmp, false, &gmtime_offset);
+#endif
}
#if defined _LIBC && __TIMESIZE != 64
diff --git a/time/tzfile.c b/time/tzfile.c
index edb5643335..98bd211bfe 100644
--- a/time/tzfile.c
+++ b/time/tzfile.c
@@ -27,6 +27,7 @@
#include <stdint.h>
#include <alloc_buffer.h>
#include <set-freeres.h>
+#include <tzset.h>
#include <timezone/tzfile.h>
diff --git a/time/tzset.c b/time/tzset.c
index 0ddc203287..b521310236 100644
--- a/time/tzset.c
+++ b/time/tzset.c
@@ -16,13 +16,13 @@
<https://www.gnu.org/licenses/>. */
#include <ctype.h>
-#include <libc-lock.h>
#include <stdbool.h>
#include <stddef.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <time.h>
+#include <tzset.h>
#include <timezone/tzfile.h>
@@ -37,7 +37,7 @@ weak_alias (__daylight, daylight)
weak_alias (__timezone, timezone)
/* This locks all the state variables in tzfile.c and this file. */
-__libc_lock_define_initialized (static, tzset_lock)
+__libc_lock_define_initialized (, __tzset_lock)
/* This structure contains all the information about a
timezone given in the POSIX standard TZ envariable. */
@@ -544,9 +544,8 @@ __tz_compute (__time64_t timer, struct tm *tm, int use_localtime)
#undef tzset
void
-__tzset (void)
+__tzset_unlocked (void)
{
- __libc_lock_lock (tzset_lock);
tzset_internal (1);
@@ -557,8 +556,16 @@ __tzset (void)
__tzname[1] = (char *) tz_rules[1].name;
}
- __libc_lock_unlock (tzset_lock);
}
+
+void
+__tzset (void)
+{
+ __libc_lock_lock (__tzset_lock);
+ __tzset_unlocked ();
+ __libc_lock_unlock (__tzset_lock);
+}
+
weak_alias (__tzset, tzset)
/* Return the `struct tm' representation of TIMER in the local timezone.
@@ -569,7 +576,6 @@ __tz_convert (__time64_t timer, int use_localtime, struct tm *tp)
long int leap_correction;
int leap_extra_secs;
- __libc_lock_lock (tzset_lock);
/* Update internal database according to current TZ setting.
POSIX.1 8.3.7.2 says that localtime_r is not required to set tzname.
@@ -589,8 +595,6 @@ __tz_convert (__time64_t timer, int use_localtime, struct tm *tp)
leap_extra_secs = 0;
}
- __libc_lock_unlock (tzset_lock);
-
if (tp)
{
if (! use_localtime)
diff --git a/time/tzset.h b/time/tzset.h
new file mode 100644
index 0000000000..254540aa05
--- /dev/null
+++ b/time/tzset.h
@@ -0,0 +1,34 @@
+#ifndef _TZSET_H
+#define _TZSET_H
+
+#include <time.h>
+#include <libc-lock.h>
+
+/* Defined in tzset.c. */
+extern char *__tzstring (const char *string) attribute_hidden;
+
+extern int __use_tzfile attribute_hidden;
+
+extern void __tzfile_read (const char *file, size_t extra,
+ char **extrap) attribute_hidden;
+extern void __tzfile_compute (__time64_t timer, int use_localtime,
+ long int *leap_correct, int *leap_hit,
+ struct tm *tp) attribute_hidden;
+extern void __tzfile_default (const char *std, const char *dst,
+ int stdoff, int dstoff)
+ attribute_hidden;
+extern void __tzset_parse_tz (const char *tz) attribute_hidden;
+extern void __tz_compute (__time64_t timer, struct tm *tm, int use_localtime)
+ __THROW attribute_hidden;
+
+__libc_lock_define (extern, __tzset_lock attribute_hidden);
+
+extern void __tzset (void) attribute_hidden;
+extern void __tzset_unlocked (void) attribute_hidden;
+
+/* Prototype for the internal function to get information based on TZ.
+ Caller needs to lock __tzset_lock. */
+extern struct tm *__tz_convert (__time64_t timer, int use_localtime,
+ struct tm *tp) attribute_hidden;
+
+#endif /* _TZSET_H */
--
2.47.1
More information about the Libc-alpha
mailing list