On 2025-09-26 10:55, Paul Eggert wrote:
I'm planning a future patch to use AT_RESOLVE_BENEATH
if available, and this won't be an issue on FreeBSD once that happens.
I did this by installing the attached patches. Comments welcome.
In doing it I noticed one tiny glitch in the FreeBSD implementation: it
opens /usr/share/zoneinfo with O_RDONLY. Surely that should be O_SEARCH.
This makes no practical difference since the directory is always both
readable and searchable, but I thought I'd mention it.
Second, even platforms lacking AT_RESOLVE_BENEATH get equivalent
security guarantees, because we can trust the contents of /usr/share/
zoneinfo (if we can't trust that directory, we have bigger problems than
those fixable by AT_RESOLVE_BENEATH!) and tzcode's localtime checks for
".." in relative pathnames (I'll arrange for this check to be optimized
away on platforms with AT_RESOLVE_BENEATH).
On thinking about it more, I still don't see how bleeding-edge FreeBSD's
recently-added openat+open approach is a win for localtime.c. That
approach slows things down (because we now have two extra syscalls:
openat and close); it introduces new failure modes (e.g., EMFILE); and
it provides no extra security because it does not fix any TOCTTOU races
in practice. Although the attached patches implement the approach anyway
(as an option), I'm hoping that upon further reflection the FreeBSD
maintainers agree that it's unnecessary; if so, we can remove the option
from tzcode.
If I'm not mistaken, the two remaining features in FreeBSD but not
tzcode are support for intermittent polling for TZif file changes, and
support for poorly written multithreaded programs that use
localtime/gmtime/offtime instead of localtime_r/gmtime_r/offtime_r. I
plan to look at these two features in that order.
From ad0716e2f0f821011f0b75e439e62deb3ce3427d Mon Sep 17 00:00:00 2001
From: Paul Eggert <[email protected]>
Date: Fri, 26 Sep 2025 15:40:04 -0700
Subject: [PROPOSED 1/3] Use open+openat (if available) for TZif access
This mimics what bleeding-edge FreeBSD does.
* Makefile, NEWS: Mention this.
* localtime.c (AT_FDCWD, O_DIRECTORY, O_PATH, O_SEARCH, OPENAT_TZDIR):
Default to 0.
(openat) [!AT_FDCWD]: New dummy function, which is never called.
(tzdirslash) [OPENAT_TZDIR]: Now null-terminated.
(tzdirslashlen): New convenience constant.
(tzloadbody) [OPENAT_TZDIR]: Use openat+AT_FDCWD if available.
---
Makefile | 2 ++
NEWS | 3 +++
localtime.c | 75 +++++++++++++++++++++++++++++++++++++++++++----------
3 files changed, 66 insertions(+), 14 deletions(-)
diff --git a/Makefile b/Makefile
index 5b577781..47d0643c 100644
--- a/Makefile
+++ b/Makefile
@@ -281,6 +281,8 @@ LDLIBS=
# -DHAVE_UTMPX_H=0 if <utmpx.h> does not work*
# -Dlocale_t=XXX if your system uses XXX instead of locale_t
# -DMKTIME_MIGHT_OVERFLOW if mktime might fail due to time_t overflow
+# -DOPENAT_TZDIR if tzset should use openat on TZDIR then a relative open.
+# See localtime.c for details.
# -DPORT_TO_C89 if tzcode should also run on mostly-C89 platforms+
# Typically it is better to use a later standard. For example,
# with GCC 4.9.4 (2016), prefer '-std=gnu11' to '-DPORT_TO_C89'.
diff --git a/NEWS b/NEWS
index 2c354038..03280c43 100644
--- a/NEWS
+++ b/NEWS
@@ -62,6 +62,9 @@ Unreleased, experimental changes
tzcode now uses strnlen to improve asymptotic performance a bit.
Compile with -DHAVE_STRNLEN=0 if your platform lacks it.
+ tzset etc. now have an experimental OPENAT_TZDIR option;
+ see Makefile and localtime.c for details.
+
Changes to commentary
The leapseconds file contains commentary about the IERS and NIST
diff --git a/localtime.c b/localtime.c
index aa9ad7bd..27b0455f 100644
--- a/localtime.c
+++ b/localtime.c
@@ -108,6 +108,12 @@ typedef intmax_t timex_t;
# endif
#endif
+/* Placeholders for platforms lacking openat. */
+#ifndef AT_FDCWD
+# define AT_FDCWD (-1) /* any negative value will do */
+static int openat(int dd, char const *path, int oflag) { unreachable (); }
+#endif
+
/* Port to platforms that lack some O_* flags. Unless otherwise
specified, the flags are standardized by POSIX. */
@@ -120,12 +126,21 @@ typedef intmax_t timex_t;
#ifndef O_CLOFORK
# define O_CLOFORK 0
#endif
+#ifndef O_DIRECTORY
+# define O_DIRECTORY 0
+#endif
#ifndef O_IGNORE_CTTY
# define O_IGNORE_CTTY 0 /* GNU/Hurd */
#endif
#ifndef O_NOCTTY
# define O_NOCTTY 0
#endif
+#ifndef O_PATH
+# define O_PATH 0
+#endif
+#ifndef O_SEARCH
+# define O_SEARCH 0
+#endif
/* Return 1 if the process is privileged, 0 otherwise. */
#if !HAVE_ISSETUGID
@@ -200,6 +215,19 @@ static char const *utc = etc_utc + sizeof "Etc/" - 1;
# define TZDEFRULESTRING ",M3.2.0,M11.1.0"
#endif
+/* If compiled with -DOPENAT_TZDIR, then when accessing a relative
+ name like "America/Los_Angeles", first open TZDIR (default
+ "/usr/share/zoneinfo") as a directory and then use the result in
+ openat with "America/Los_Angeles", rather than the traditional
+ approach of opening "/usr/share/zoneinfo/America/Los_Angeles".
+ Although the OPENAT_TZDIR approach is less efficient, suffers from
+ spurious EMFILE and ENFILE failures, and is no more secure in practice,
+ it is how bleeding edge FreeBSD did things from August 2025
+ through at least September 2025. */
+#ifndef OPENAT_TZDIR
+# define OPENAT_TZDIR 0
+#endif
+
/* If compiled with -DSUPPRESS_TZDIR, do not prepend TZDIR to relative TZ.
This is intended for specialized applications only, due to its
security implications. */
@@ -539,8 +567,9 @@ union input_buffer {
+ 4 * TZ_MAX_TIMES];
};
-/* TZDIR with a trailing '/' rather than a trailing '\0'. */
-static char const tzdirslash[sizeof TZDIR] = TZDIR "/";
+/* TZDIR with a trailing '/'. It is null-terminated if OPENAT_TZDIR. */
+static char const tzdirslash[sizeof TZDIR + OPENAT_TZDIR] = TZDIR "/";
+static size_t const tzdirslashlen = sizeof TZDIR;
/* Local storage needed for 'tzloadbody'. */
union local_storage {
@@ -580,6 +609,10 @@ tzloadbody(char const *name, struct state *sp, char tzloadflags,
char const *relname;
register union input_buffer *up = &lsp->u.u;
register int tzheadsize = sizeof(struct tzhead);
+ int dd = AT_FDCWD;
+ int oflags = (O_RDONLY | O_BINARY | O_CLOEXEC | O_CLOFORK
+ | O_IGNORE_CTTY | O_NOCTTY);
+ int open_err;
sp->goback = sp->goahead = false;
@@ -598,8 +631,8 @@ tzloadbody(char const *name, struct state *sp, char tzloadflags,
subsidiary to TZDIR. Also, NAME is not a device. */
if (name[0] == '/' && strcmp(name, TZDEFAULT) != 0) {
if (!SUPPRESS_TZDIR
- && strncmp(relname, tzdirslash, sizeof tzdirslash) == 0)
- for (relname += sizeof tzdirslash; *relname == '/'; relname++)
+ && strncmp(relname, tzdirslash, tzdirslashlen) == 0)
+ for (relname += tzdirslashlen; *relname == '/'; relname++)
continue;
else if (issetugid())
return ENOTCAPABLE;
@@ -618,18 +651,30 @@ tzloadbody(char const *name, struct state *sp, char tzloadflags,
}
}
- /* Fail if a relative name contains a ".." component,
- as such a name could read a file outside TZDIR. */
if (relname[0] != '/') {
+ /* Fail if a relative name contains a ".." component,
+ as such a name could read a file outside TZDIR. */
char const *component;
for (component = relname; component[0]; component++)
if (component[0] == '.' && component[1] == '.'
&& ((component[2] == '/') | !component[2])
&& (component == relname || component[-1] == '/'))
return ENOTCAPABLE;
+
+ if (OPENAT_TZDIR) {
+ /* Prefer O_SEARCH or O_PATH if available;
+ O_RDONLY should be OK too, as TZDIR is invariably readable.
+ O_DIRECTORY should be redundant but might help
+ on old platforms that mishandle trailing '/'. */
+ dd = open(tzdirslash,
+ ((O_SEARCH ? O_SEARCH : O_PATH ? O_PATH : O_RDONLY)
+ | O_BINARY | O_CLOEXEC | O_CLOFORK | O_DIRECTORY));
+ if (dd < 0)
+ return errno;
+ }
}
- if (!SUPPRESS_TZDIR && name[0] != '/') {
+ if (!OPENAT_TZDIR && !SUPPRESS_TZDIR && name[0] != '/') {
char *cp;
size_t namesizemax = sizeof lsp->fullname - sizeof tzdirslash;
size_t namelen = strnlen (name, namesizemax);
@@ -640,16 +685,18 @@ tzloadbody(char const *name, struct state *sp, char tzloadflags,
would pull in stdio (and would fail if the
resulting string length exceeded INT_MAX!). */
cp = lsp->fullname;
- cp = mempcpy(cp, tzdirslash, sizeof tzdirslash);
+ cp = mempcpy(cp, tzdirslash, tzdirslashlen);
cp = mempcpy(cp, name, namelen);
*cp = '\0';
name = lsp->fullname;
}
- fid = open(name, (O_RDONLY | O_BINARY | O_CLOEXEC | O_CLOFORK
- | O_IGNORE_CTTY | O_NOCTTY));
+ fid = OPENAT_TZDIR ? openat(dd, relname, oflags) : open(name, oflags);
+ open_err = errno;
+ if (0 <= dd)
+ close(dd);
if (fid < 0)
- return errno;
+ return open_err;
nread = read(fid, up->buf, sizeof up->buf);
if (nread < tzheadsize) {
@@ -1543,9 +1590,9 @@ tzset_unlocked(void)
/* Abbreviate a string like "/usr/share/zoneinfo/America/Los_Angeles"
to its shorter equivalent "America/Los_Angeles". */
- if (!SUPPRESS_TZDIR && sizeof tzdirslash < namelen
- && memcmp(name, tzdirslash, sizeof tzdirslash) == 0) {
- char const *p = name + sizeof tzdirslash;
+ if (!SUPPRESS_TZDIR && tzdirslashlen < namelen
+ && memcmp(name, tzdirslash, tzdirslashlen) == 0) {
+ char const *p = name + tzdirslashlen;
while (*p == '/')
p++;
if (*p && *p != ':') {
--
2.48.1
From fd46fea69707d535da268fa02277b64117819fa7 Mon Sep 17 00:00:00 2001
From: Paul Eggert <[email protected]>
Date: Fri, 26 Sep 2025 16:22:47 -0700
Subject: [PROPOSED 2/3] Use O_RESOLVE_BENEATH (if available) for TZif
* localtime.c (O_RESOLVE_BENEATH): Default to 0.
(tzloadbody): If O_RESOLVE_BENEATH is available, use it
instead of manually checking for "..".
This mimics what bleeding-edge FreeBSD does.
---
localtime.c | 23 +++++++++++++++--------
1 file changed, 15 insertions(+), 8 deletions(-)
diff --git a/localtime.c b/localtime.c
index 27b0455f..c103909a 100644
--- a/localtime.c
+++ b/localtime.c
@@ -138,6 +138,9 @@ static int openat(int dd, char const *path, int oflag) { unreachable (); }
#ifndef O_PATH
# define O_PATH 0
#endif
+#ifndef O_RESOLVE_BENEATH
+# define O_RESOLVE_BENEATH 0
+#endif
#ifndef O_SEARCH
# define O_SEARCH 0
#endif
@@ -652,14 +655,17 @@ tzloadbody(char const *name, struct state *sp, char tzloadflags,
}
if (relname[0] != '/') {
- /* Fail if a relative name contains a ".." component,
- as such a name could read a file outside TZDIR. */
- char const *component;
- for (component = relname; component[0]; component++)
- if (component[0] == '.' && component[1] == '.'
- && ((component[2] == '/') | !component[2])
- && (component == relname || component[-1] == '/'))
- return ENOTCAPABLE;
+ if (!OPENAT_TZDIR || !O_RESOLVE_BENEATH) {
+ /* Fail if a relative name contains a ".." component,
+ as such a name could read a file outside TZDIR
+ when AT_FDCWD and O_RESOLVE_BENEATH are not available. */
+ char const *component;
+ for (component = relname; component[0]; component++)
+ if (component[0] == '.' && component[1] == '.'
+ && ((component[2] == '/') | !component[2])
+ && (component == relname || component[-1] == '/'))
+ return ENOTCAPABLE;
+ }
if (OPENAT_TZDIR) {
/* Prefer O_SEARCH or O_PATH if available;
@@ -671,6 +677,7 @@ tzloadbody(char const *name, struct state *sp, char tzloadflags,
| O_BINARY | O_CLOEXEC | O_CLOFORK | O_DIRECTORY));
if (dd < 0)
return errno;
+ oflags |= O_RESOLVE_BENEATH;
}
}
--
2.48.1
From 997441bed2d4832e0ee5f9effc037986b725059c Mon Sep 17 00:00:00 2001
From: Paul Eggert <[email protected]>
Date: Sun, 28 Sep 2025 23:05:20 -0700
Subject: [PROPOSED 3/3] Fix two off-by-1 nits
* localtime.c (union local_storage, tzloadbody): Fix two
recently-introduced off-by-one gotchas. Neither of these caused a bug,
but we might as well do the calculations correctly.
---
localtime.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/localtime.c b/localtime.c
index c103909a..3937e5dd 100644
--- a/localtime.c
+++ b/localtime.c
@@ -591,7 +591,7 @@ union local_storage {
However, there is no need for this to be smaller than struct
file_analysis as that struct is allocated anyway, as the other
union member. */
- char fullname[max(sizeof(struct file_analysis), sizeof tzdirslash + 1024)];
+ char fullname[max(sizeof(struct file_analysis), sizeof TZDIR + 1024)];
};
/* These tzload flags can be ORed together, and fit into 'char'. */
@@ -683,7 +683,7 @@ tzloadbody(char const *name, struct state *sp, char tzloadflags,
if (!OPENAT_TZDIR && !SUPPRESS_TZDIR && name[0] != '/') {
char *cp;
- size_t namesizemax = sizeof lsp->fullname - sizeof tzdirslash;
+ size_t namesizemax = sizeof lsp->fullname - tzdirslashlen;
size_t namelen = strnlen (name, namesizemax);
if (namesizemax <= namelen)
return ENAMETOOLONG;
--
2.48.1
